Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

make the sidebare full-width on small screens #2747

Merged
merged 2 commits into from
Jun 10, 2022

Conversation

szaimen
Copy link
Contributor

@szaimen szaimen commented Jun 9, 2022

Close nextcloud/calendar#4161

Before After
image image

Signed-off-by: szaimen szaimen@e.mail.de

Signed-off-by: szaimen <szaimen@e.mail.de>
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).

Signed-off-by: szaimen <szaimen@e.mail.de>
@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
6 participants