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

fixed navbar bug on small devices #216

Open
wants to merge 3 commits into
base: development
Choose a base branch
from

Conversation

tushar912
Copy link

I have made this commit for the issue #211 regarding the navbar on small devices

@netlify
Copy link

netlify bot commented Aug 12, 2020

Deploy preview for codeuino processing.

Building with commit 0a986a0

https://app.netlify.com/sites/codeuino/deploys/5f39487135a421000885f6fb

Copy link
Member

@DevanshCodes DevanshCodes left a comment

Choose a reason for hiding this comment

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

Hey! I can only see one of the changes out of 3 which were mentioned on the issue. Please look into the other two also.

Copy link
Member

@DevanshCodes DevanshCodes left a comment

Choose a reason for hiding this comment

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

For 1st point in the issue, the implementation is correct. Good Work! 🚀

@tushar912
Copy link
Author

I have changes for 3 mentioned in issue to push nav right .....i did not get what is exactly meant by part 2.....can u explain it.

@DevanshCodes
Copy link
Member

I have changes for 3 mentioned in issue to push nav right .....i did not get what is exactly meant by part 2.....can u explain it.

@ksraj123 can you explain him what you meant by 2nd comment in the issue you opened.

@ksraj123
Copy link
Collaborator

Hi, when the navbar opens then a backdrop should appear clicking anywhere on the backdrop should close the navbar.
The user should not be able to navigate or scroll through the page with the navbar open its not good user experience.

Let's have a look at how it is now -

Peek 2020-08-16 12-01

The user can easily scroll through the page and even click on links such as Team or Code of Conduct without the navbar ever closing.

Hint - we are already using material-ui you can use Material UI Drawer to implement both the side navbar and backdrop functionality as your current side navbar does not look very nice. We want the material drawer only for screen width below which the navbar collapses

Here is how material drawer looks

Peek 2020-08-16 12-09

In our case, clicking on the hamburger icon will make the drawer appear, and clicking on the backdrop will close it.

@ksraj123
Copy link
Collaborator

Please exclude package-lock.json from your PR. Also, maybe you could implement this after you pull the latest changes after PR #214 gets merged otherwise there could be merge conflicts and that PR changes a lot so there could be some more work required on the navbar part.

@tushar912
Copy link
Author

ok sure.Thanks!

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 this pull request may close these issues.

None yet

3 participants