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

Vertical Nav add negative margin on mobile view with menubar-vertical-expand-lg class #5521

Closed
ethib137 opened this issue May 10, 2023 · 5 comments · Fixed by #5556
Closed
Assignees
Labels
comp: clay-css Issues related to Clay CSS type: bug Issues reporting that Component is not doing what should be done

Comments

@ethib137
Copy link
Member

Is there an example you can provide via codesandbox.com?

https://codesandbox.io/s/vertical-nav-margin-issues-g9uex8?file=/src/index.js

What are the steps to reproduce?

  1. Go to https://codesandbox.io/s/vertical-nav-margin-issues-g9uex8?file=/src/index.js
  2. Make sure the window is small enough to display the mobile view.

What is the expected result?

The menu and trigger should fit inside the window.

Actual Result:
The menu and trigger have negative margin and are overlapped by the window.

Screenshot 2023-05-10 at 4 30 10 PM

@ethib137 ethib137 added the type: bug Issues reporting that Component is not doing what should be done label May 10, 2023
@pat270 pat270 added the comp: clay-css Issues related to Clay CSS label May 10, 2023
@pat270
Copy link
Member

pat270 commented May 10, 2023

@ethib137 We were assuming a vertical nav would always be inside a grid column which has 12px gutters. Do we need both? Negative margins when inside a grid column and no margins when not?

@pat270
Copy link
Member

pat270 commented May 10, 2023

@ethib137 There are issues with negative margin, but I think we need to have a discussion with @drakonux, @emiliano-cicero and @marcoscv-work about how the vertical nav should be positioned in mobile especially with other items like a search input. In the current state, it's supposed to be another navigation like bar in mobile.

menubar

@ethib137
Copy link
Member Author

@emiliano-cicero , has this already been discussed how this should look on small screens / mobile?

@emiliano-cicero
Copy link

I don't think it was discussed directly but we probably assumed having a default margin given by the container. The example for the vertical-nav on mobile has no-margin because it's aligned to the form and as @pat270 says, I think it's probably the 12px gutter (although visually it seems more like 16px so there might be something else).
image

If it's placed without a container then it should respect the margins from both sides.

Regarding the position, I don't know if I understood correctly but I'll try to answer: the vertical nav should be considered almost as part of the content since the interaction is directly connected with the content on the left/bottom, the search bar would go on top as is part of the management toolbar (with its mobile version).

@github-actions
Copy link

This issue has been merged and will be released in DXP at https://issues.liferay.com/browse/LPS-187558

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
comp: clay-css Issues related to Clay CSS type: bug Issues reporting that Component is not doing what should be done
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants