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.

Sign up for GitHub

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

Jump to bottom

Fix menu mobile #100

Merged
merged 6 commits into from
Oct 10, 2023
Merged

Fix menu mobile #100

merged 6 commits into from
Oct 10, 2023

Conversation

giladfuchs
Copy link
Collaborator

Resolve #64
Bug - Open Menu In the Mobile Display #64

Description

small css edit

screenshots

Screen Shot 2023-10-06 at 17 37 56

Screen Shot 2023-10-06 at 17 39 31

giladfuchs added 3 commits October 5, 2023 19:13
fix in menu.scss for
f07cf35 
Bug - Open Menu In the Mobile Display hasadna#64
fix in menu.scss for
277c88a 
Bug - Open Menu In the Mobile Display hasadna#64
fix in menu.scss for
661ab12 
Bug - Open Menu In the Mobile Display hasadna#64
NoamGaash
NoamGaash reviewed Oct 6, 2023
View reviewed changes
src/pages/components/header/menu/menu.scss Outdated
max-width: 100%;
padding: 30px;
margin-right: .51rem;
Copy link
Member

Choose a reason for hiding this comment

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

why this is the right number?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

it's can be change to 0.5 rem.

Copy link
Member

@NoamGaash NoamGaash Oct 7, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

rem is related to the default font size, so I'm curios fo understand why this specific size has been chosen

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm use to work with rem ..
any way I change it to px

Copy link
Member

Choose a reason for hiding this comment

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

rem is fine - I'm just confused by the very specific number

@NoamGaash
Copy link
Member

@giladfuchs
please use the devtool to test various devices

image

@giladfuchs
Copy link
Collaborator Author

this pr it's not about the nav button section.

it's about the burger section.
when you open it on mobile it's go to the right side and you can't see it
Screen Shot 2023-10-10 at 13 22 18

cause the screen get extend and it's not sticky.

so you need to go to the right to see it

Screen Shot 2023-10-10 at 13 24 13
and after you try to close it this is what you get

Screen Shot 2023-10-10 at 13 26 19

this pr fix it to look like that.

Screen Shot 2023-10-10 at 13 26 50

and after you close it. it back to normal!

@NoamGaash
Copy link
Member

allright, I see - thank you
however, let's open another issue to make this menu really mobile-friendly :)

@NoamGaash NoamGaash merged commit 5642449 into hasadna:main Oct 10, 2023
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@NoamGaash NoamGaash NoamGaash approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Bug - Open Menu In the Mobile Display
2 participants
@giladfuchs @NoamGaash