-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Improve Navigation Layout and UX Enhancement #1518
Conversation
modified: css/dropit.css modified: css/style.css modified: css/theme.css new file: images/ui/element/moon-icon.svg new file: images/ui/element/moon-icon@2x.svg new file: images/ui/element/nnnn.svg new file: images/ui/element/search-icon.svg new file: images/ui/element/search-icon@2x.svg new file: images/ui/element/sun-icon.svg new file: images/ui/element/sun-icon@2x.svg modified: js/theme.js new file: package-lock.json
modified: css/style.css
deleted: images/ui/element/moon-icon.svg deleted: images/ui/element/moon-icon@2x.svg deleted: images/ui/element/nnnn.svg deleted: images/ui/element/search-icon.svg deleted: images/ui/element/search-icon@2x.svg deleted: images/ui/element/sun-icon.svg deleted: images/ui/element/sun-icon@2x.svg modified: js/theme.js
modified: css/dropit.css modified: css/style.css modified: css/theme.css new file: images/hamburger-dark.svg new file: images/hamburger-light.svg new file: images/up-arrow-dark.svg new file: images/up-arrow-light.svg modified: js/dropit.js
modified: _includes/header/header-es.html modified: _includes/header/header-fr.html modified: _includes/header/header-it.html modified: _includes/header/header-ja.html modified: _includes/header/header-ko.html modified: _includes/header/header-pt-br.html modified: _includes/header/header-ru.html modified: _includes/header/header-sk.html modified: _includes/header/header-th.html modified: _includes/header/header-tr.html modified: _includes/header/header-uk.html modified: _includes/header/header-uz.html modified: _includes/header/header-zh-cn.html modified: _includes/header/header-zh-tw.html modified: css/theme.css
modified: js/dropit.js
✅ Deploy Preview for expressjscom-preview ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Thanks for @OmkarSonawane1230. A quick look over a few of the changes and I found a few things to comment on.
Overall this is just too many file changes all at once and too many unsolicited changes. I didn't go through every single feature you suggested, and there might be some good stuff , but a better way might be to suggest a change or two individually first, and then once they've been approved you can implement them. This would make it much easier to review too, dealing with less files and changes at once. And also, if the change isn't approved, it saves you the the time of working on it. For example, just navbar itself could perhaps benefit from a revamp. Just this alone might be one PR, or even just parts of the navbar per PR depending on the scale of the changes. Rather than overhauling the site navigation and UX all in one go. This is just my 5 cents. What do you think of that @crandmck? |
Hi @chrisdel101, Thanks for taking the time to review my pull request. I appreciate your feedback and suggestions for improving the navigation. I want to highlight some points you mentioned.
I agree that the UI changes are to much for current UI, but my I idea is to redesign the Expressjs.com. I decided to do following things.
@crandmck and @chrisdel101 |
Thanks @OmkarSonawane1230 for the PR, and thanks also @chrisdel101 for reviewing it. I agree with @chrisdel101 that there are a lot of changes in a single PR. In general it's best if a PR addresses an existing issue which has ideally had some prior discussion. This one is a bit "out of the blue" and objectively it's not clear what it accomplishes.
I appreciate the sentiment and enthusiasm, and while the site is not perfect by any means, it's not clear that it needs a complete redesign, and there are other issues with the content (vs. layout/design) that are higher priority IMO.
Hard to argue with that, but what specifically needs to be improved and why? "UI improvement" can often be a matter of opinion and IMO should be the result of a collaborative & comprehensive design effort.
What does this mean? It seems well formatted to me, but this could mean almost anything.
This is a big change, and would require discussion by the @expressjs/express-tc. The biggest issue that I see is that the TC and project collaborators more broadly have more than enough on their plate already trying to release 5.x and definitely don't have time to act as a "help desk" to the entire Express community, as much as we might like to do that. I'm open to the idea of a redesign in general, but I'd like to understand the value added, beyond qualitative or purely subjective rationale. I'm going to close this issue, but I'm open to considering INCREMENTAL changes along these lines in the future. I would ask that you open issues in the repo identifying specific, well-defined problems or deficiencies and then open PR(s) to address those... This will just make it easier for us to manage the changes and ensure that they improve the overall quality of the documentation. @OmkarSonawane1230 I do appreciate your efforts and I don't want to discourage you, but I hope you'll understand that we want to stay focused on improving the overall quality of the documentation, and not making sweeping changes without due diligence. |
Improve Navigation Layout, Responsiveness and UX Enhancement
Redesign the layout of navigation to improve the UX and responsiveness of our navigation system while ensuring seamless integration with translations.
Key Enhancements:
Navigation Layout Redesign:
Improve Responsiveness:
Apply the Changes to all Translation:
Improved UX by adding some animation:
Screen.Recording.2024-05-18.at.7.10.37.PM.mov
Screenshots