-
-
Notifications
You must be signed in to change notification settings - Fork 211
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]: Fixed Navbar dark mode navigation #1059
Conversation
In macOS, I'm encountering another issue with the Navbar search button. The search button's style contains a background-color property. |
built with Refined Cloudflare Pages Action⚡ Cloudflare Pages Deployment
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1059 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 10 10
Lines 373 373
Branches 94 94
=========================================
Hits 373 373 ☔ View full report in Codecov by Sentry. |
Bumps [husky](https://github.com/typicode/husky) from 9.1.5 to 9.1.6. - [Release notes](https://github.com/typicode/husky/releases) - [Commits](typicode/husky@v9.1.5...v9.1.6) --- updated-dependencies: - dependency-name: husky dependency-type: direct:development update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> fixed navbar dark mode navigation fixed fixed
d495e6d
to
1dfe08f
Compare
I'm sorry for making this pull request complex for the reviewer. Initially, I did my work well, but the |
No worries with that just remove the changes from |
Yeah I did 😄 |
The commit #1049 is also included in my pull request. |
@DhairyaMajmudar Please Review Code now. |
@officeneerajsaini pls. don't change the font colors |
In dark mode, some light colors look better than a dark-on-dark combination. Consistent color: Screen.Recording.2024-10-25.at.11.26.19.AM.movSome Lightness : Screen.Recording.2024-10-25.at.11.25.50.AM.movShould I go with the website theme color? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great, Thanks!!
However I do prefer the white color for the navbar fonts instead that light grey. Can we change that?
Screen.Recording.2024-10-30.at.5.46.20.PM.mov |
@benjagm now take a look on code |
It looks better. However, the blue color is difficult to distinguish from the background. Can we find something lighter? Why not just underlying the current section instead of changing the color? |
Screen.Recording.2024-11-08.at.4.18.13.PM.mov |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems like this class is no longer used, if it is so you can remove it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dhairya, I changed the grey color to white in response to Benjamin's request.
Looks great, Thanks!!
However I do prefer the white color for the navbar fonts instead that light grey. Can we change that?
I also added a new CSS property to show an underline on the current page, so some additional CSS properties need to be added. 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awsm! With that then keep it don't remove
Hi everyone. I checked the current version and found this behavior:
|
Yes , Found that part too . |
@officeneerajsaini we are very close! Let's make the last change and push it. Great work so far. Thanks for your patience. |
Hi @officeneerajsaini . Is there any update on this? |
Screen.Recording.2024-12-13.at.7.28.53.PM.movI fixed the last issue related to the Tools navigation behaviour. This PR is now ready to merge. Here’s a summary of what has been done in this PR:
|
Thanks @officeneerajsaini . Ok I think the reason why the preview deployment I checked is outdated is because the deployment work failed due to some format checks: https://github.com/json-schema-org/website/actions/runs/11986321387/job/33419247261?pr=1059 Can you run prettier on that file? I am so looking forward to merge it. thanks for your patience. It has been a very busy weeks for me on my new job. |
@benjagm Yeah, I know you were very busy at that time, which is why I waited for your response. |
Now This Code Ready To Merge @benjagm Please take a Look. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the hard work Neeraj. I just added a small change to make it work. Kudos!
Thanks, Benja 🫠 |
This Pull Request Fixes the Navbar Navigation Color in Dark mode.
Issue Number:
Screenshots/videos:
Before:
Screen.Recording.2024-10-24.at.10.51.20.AM.mov
After :
Screen.Recording.2024-10-24.at.10.52.49.AM.mov