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

fix(mega-menu): Mega menu content position fix - FRONT-4606 #3615

Merged
merged 14 commits into from
Sep 26, 2024

Conversation

planctus
Copy link
Contributor

@planctus planctus commented Sep 9, 2024

No description provided.

Copy link

github-actions bot commented Sep 9, 2024

@github-actions github-actions bot temporarily deployed to pull request September 9, 2024 13:15 Inactive
@emeryro
Copy link
Contributor

emeryro commented Sep 10, 2024

I get the idea behind this task, but the result design is quite odd (site header has some padding while the mega menu takes 100% width)
image
image

Maybe we could keep the scrollbar, but just "disable" it. I found this for instance, that seems to work:

.noscroll {
    position: fixed; 
    overflow-y: scroll;
    width: 100%;
}

https://stackoverflow.com/questions/8701754/how-to-disable-scroll-without-hiding-it

@planctus
Copy link
Contributor Author

mmh..i tried with position: fixed already but i gave up for a reason: in desktop you could open the menu after having scrolled the page a bit, using position: fixed will reset that scroll and bring the page back to top: 0;
I've tried to get the scroll position but maybe because we are in an iframe in our demo that returned always 0.

I haven't noticed myself the issue with the padding being set on the body and the menu still getting the full width, so i chose this solution, but comparing the two it seems a minor issue the one described before with the position fixed, so let's go with it

@planctus planctus removed the Question label Sep 10, 2024
@github-actions github-actions bot temporarily deployed to pull request September 10, 2024 10:35 Inactive
@emeryro
Copy link
Contributor

emeryro commented Sep 11, 2024

The fix is fine, so to me this task can be merged already.
I just have one additional suggestion: in real cases, the display is good, but on our demo the site header is still "jumping" as we don't have scrollbar here. Maybe we could update the demo to always have a scrollbar for this component

@github-actions github-actions bot temporarily deployed to pull request September 11, 2024 10:35 Inactive
@planctus planctus changed the base branch from v4.6.4-dev to v4-dev September 12, 2024 13:48
@github-actions github-actions bot temporarily deployed to pull request September 12, 2024 13:59 Inactive
@github-actions github-actions bot temporarily deployed to pull request September 13, 2024 08:18 Inactive
@github-actions github-actions bot temporarily deployed to pull request September 13, 2024 08:56 Inactive
@github-actions github-actions bot temporarily deployed to pull request September 16, 2024 15:07 Inactive
…m:ec-europa/europa-component-library into FRONT-4606-Mega-menu-content-position-fix
@github-actions github-actions bot temporarily deployed to pull request September 18, 2024 14:04 Inactive
@github-actions github-actions bot temporarily deployed to pull request September 24, 2024 07:03 Inactive
@github-actions github-actions bot temporarily deployed to pull request September 24, 2024 15:17 Inactive
@planctus planctus merged commit fc1e9ed into v4-dev Sep 26, 2024
7 checks passed
@planctus planctus deleted the FRONT-4606-Mega-menu-content-position-fix branch September 26, 2024 08:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants