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

[Mega menu] Center the mega menu when the header has the menu centered #1480

Closed
tyleralsbury opened this issue Mar 10, 2022 · 1 comment · Fixed by #1482
Closed

[Mega menu] Center the mega menu when the header has the menu centered #1480

tyleralsbury opened this issue Mar 10, 2022 · 1 comment · Fixed by #1482
Assignees

Comments

@tyleralsbury
Copy link
Contributor

tyleralsbury commented Mar 10, 2022

Overview

When the merchant has selected for their menu to be centered, the mega menu should have its links centered as well. See screenshot.

Screenshot

⚠️ Note that in this screenshot, the menu is not center aligned, I was just testing to see how viable it was to center the mega menu. We only want this to happen when the menu is center aligned.

Screen Shot 2022-03-10 at 9 19 30 AM

Technical details.

This one is a bit tricky. It's difficult to accomplish with the CSS grid. I have two ideas as to how we might be able to achieve this.

Flexbox

Instead of using grid, we could use display: flex; and use justify-content: center. This might cause other things to become misaligned unless we also use fixed with columns with Flexbox. We'll need to disable both flex-grow and flex-shrink to make it work. It's a shame, because I do like the way grid is being used.

Grid

We might be able to make it work with Grid... Something along the lines having the grid have a width of n/6% where n == number_of_columns | at_most: 6. We'd also need the number of columns the grid itself has to be number_of_columns | at_most: 6. This means more work being done in the Liquid itself, but might result in cleaner CSS and less of an overhaul of the code that's already there and has been tested. The grid container could get margin: auto if the menu style is centered.

I think the gird approach is better, but I haven't actually tested it. There might be something I'm missing. 🤔

@sofiamatulis sofiamatulis self-assigned this Mar 11, 2022
@sofiamatulis sofiamatulis mentioned this issue Mar 11, 2022
9 tasks
@sofiamatulis
Copy link
Contributor

Update:

I tried to use the Grid approach Tyler mentioned in the issue (For instance, 3 columns would be be 3 / 6 which would be 0.5 so 50%. https://screenshot.click/11-11-xf1z1-qymjl.png . The problem is when we have multiple rows: https://screenshot.click/11-11-0p7vv-410al.png . We'd want to ensure these rows are also centered and it'd require more logic. I also attempted to use grid-column-start but had the same issues when we have more rows.

After some research as well, grid isn't the right tool for the job since it's also trying to ensure the items stay within their grid position. Flex makes more sense at the end, especially since we have dynamic content.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants