Skip to content

Conversation

@szaimen
Copy link
Contributor

@szaimen szaimen commented Jun 9, 2022

Close nextcloud/calendar#4161

Before After
image image

Signed-off-by: szaimen [email protected]

Copy link
Contributor

@raimund-schluessler raimund-schluessler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to hide the appnavigation toggle on the top left then, otherwise the tertiary actions of the sidebar become unusable.

See Tasks for example, the toggle overlays the complete checkbox:
Screenshot 2022-06-10 at 09-04-54 Aufgaben - Nextcloud

I think in this case it's fine to have the toggle behind the sidebar. You need to close the sidebar to open the navigation, but that keeps the sidebar usable.

@raimund-schluessler raimund-schluessler added this to the 6.0.0 milestone Jun 10, 2022
@szaimen
Copy link
Contributor Author

szaimen commented Jun 10, 2022

@raimund-schluessler I suppose this needs to be done in javascript since the css is scoped?

@raimund-schluessler
Copy link
Contributor

@raimund-schluessler I suppose this needs to be done in javascript since the css is scoped?

Not necessarily, you can either use ::v-deep at the root level, or add a second style block without the scope attribute like <style lang="scss">. The question rather is what's the proper CSS code to make it work 😄

@raimund-schluessler
Copy link
Contributor

@raimund-schluessler I suppose this needs to be done in javascript since the css is scoped?

Not necessarily, you can either use ::v-deep at the root level, or add a second style block without the scope attribute like <style lang="scss">. The question rather is what's the proper CSS code to make it work 😄

Reducing the z-index of the AppNavigation to e.g. 1400, a value lower than the one of AppSidebar (1500) could be an option to fix it:
https://github.com/nextcloud/nextcloud-vue/blob/befbd70453aad32657686c8a3ddf2250fe1a5490/src/components/AppNavigation/AppNavigation.vue#L151

But this might have side effects. The result would be that AppSidebar would always be on top of AppNavigation. On large screens, I think that's no issue, since they won't overlap. On small screens (width < breakpoint-mobile/2, i.e. 512 px), it could be fine as well, since AppSidebar will just take the whole width and you need to close the sidebar to open navigation. Only on intermediate screen with a width between 512 px and 600 px (min-width of navigation 300 px plus min-width of sidebar 300 px) the overlap might be an issue.

@raimund-schluessler
Copy link
Contributor

raimund-schluessler commented Jun 10, 2022

Or, one leaves this to the app developer to hide the toggle if the sidebar is open on small screens 🙈. Because from the nextcloud-vue components, we have no easy way to figure out whether the sidebar is open (visible) or not (but I would very well be missing something).

@szaimen
Copy link
Contributor Author

szaimen commented Jun 10, 2022

What about cd98e1d @raimund-schluessler ? :)

@raimund-schluessler
Copy link
Contributor

What about cd98e1d @raimund-schluessler ? :)

Nice, good idea. I didn't test it though.

@szaimen szaimen added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Jun 10, 2022
Copy link
Contributor

@st3iny st3iny left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested and works.

@marcoambrosini marcoambrosini merged commit f4ff0ba into master Jun 10, 2022
@marcoambrosini marcoambrosini deleted the enh/noid/full-width branch June 10, 2022 11:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

4. to release Ready to be released and/or waiting for tests to finish enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Extend the sidebar editor to full width on small screens

7 participants