Layout/no issue number responsive navbar#123
Layout/no issue number responsive navbar#123santiagosj wants to merge 4 commits intofactn:masterfrom
Conversation
|
Deploy preview for resilience-landing-page ready! Built with commit 16dd1dc https://deploy-preview-123--resilience-landing-page.netlify.app
|
jwu910
left a comment
jwu910
left a comment
There was a problem hiding this comment.
Hey @santiagosj Just skimming through this PR, can you take a look at your source code formatting?
Be sure to read through the Contributing docs as well.
You should be ignoring .vscode/ at your machine level and these configs are local to your machine and should never be committed to a repository you don't own
| className={`siteNav`} | ||
| > | ||
| { navItems && navItems.map(item => ( | ||
| <Link className={item.name === 'Get Resilience' ? 'siteNav-item button primary' : 'siteNav-item'} to={item.link}> |
There was a problem hiding this comment.
We should be providing a key for each item returned in this loop
| // "xs": 400, | ||
| // "sm": 600, | ||
| // "md": 960, | ||
| // "md": 1025, // changed from 960 |
There was a problem hiding this comment.
We should probably stick to the standard breakpoints. Mock ups are mock ups. The mock up isn't responsive so it would be okay to afford some flexibility for the designer. I would not expect the volunteers to have a design for every breakpoint.
We also do not need to comment to let anyone know this was changed form 960 because it will be available in the blame.
There was a problem hiding this comment.
Let me correct myself. 1024 is a standard breakpoint,
But the original developer had set these breakpoints up according to material-ui expectations.
I think we'll want to check with design about this, to see what our experience will be like. But if we stick with material-ui breakpoints, I don't expect it being much of an issue.
|
|
||
|
|
||
|
|
||
|
|
||
|
|
||
|
|
||
|
|
There was a problem hiding this comment.
Be sure to install your own local formatters and linters. We have our own set of configs for prettier and eslint that should be followed so we're not introducing simple formatting issues.
2 participants
Here is the responsive navbar, it can be improved, I think its weak point is the landscape view.
Changes:
I made a small change in the breakpoints on the 'md' variable, from 960 to 1025, because the navbar itself asked for it. I tried to keep it as simple as possible, to facilitate its maintenance.