json-schema-org / website

JSON Schema website
https://json-schema.org
Other
67 stars 177 forks source link

[Fix]: Fixed Navbar dark mode navigation #1059

Open officeneerajsaini opened 1 month ago

officeneerajsaini commented 1 month ago

This Pull Request Fixes the Navbar Navigation Color in Dark mode.

Issue Number:

Screenshots/videos: Before:

https://github.com/user-attachments/assets/eeec6f55-fa84-4c9a-83ee-2658141c4594

After :

https://github.com/user-attachments/assets/9bcd0f82-f0b8-4259-841b-596bf9390f2a

officeneerajsaini commented 1 month ago

In macOS, I'm encountering another issue with the Navbar search button. The search button's style contains a background-color property.

officeneerajsaini commented 1 month ago
Screenshot 2024-10-24 at 11 03 04 AM
github-actions[bot] commented 1 month ago
built with Refined Cloudflare Pages Action

⚡ Cloudflare Pages Deployment

Name Status Preview Last Commit
website ✅ Ready (View Log) Visit Preview 1923643b1795804c538646e0003d56212ba97f95
codecov[bot] commented 1 month ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 100.00%. Comparing base (7ecd915) to head (1923643). Report is 23 commits behind head on main.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #1059 +/- ## ========================================= Coverage 100.00% 100.00% ========================================= Files 10 10 Lines 373 373 Branches 94 94 ========================================= Hits 373 373 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

officeneerajsaini commented 1 month ago

I'm sorry for making this pull request complex for the reviewer. Initially, I did my work well, but the package-lock.json and yarn.lock files were included. Then I made another commit, and I wanted to combine both commits into a single one. Due to some issues, I deleted my branch and then restored it. Now, I've combined both commits into one, but it also includes the previous merged commit. That's what has been happening with me.

DhairyaMajmudar commented 1 month ago

I'm sorry for making this pull request complex for the reviewer. Initially, I did my work well, but the package-lock.json and yarn.lock files were included. Then I made another commit, and I wanted to combine both commits into a single one. Due to some issues, I deleted my branch and then restored it. Now, I've combined both commits into one, but it also includes the previous merged commit. That's what has been happening with me.

No worries with that just remove the changes from yarn.lock and package.json files.

officeneerajsaini commented 1 month ago

I'm sorry for making this pull request complex for the reviewer. Initially, I did my work well, but the package-lock.json and yarn.lock files were included. Then I made another commit, and I wanted to combine both commits into a single one. Due to some issues, I deleted my branch and then restored it. Now, I've combined both commits into one, but it also includes the previous merged commit. That's what has been happening with me.

No worries with that just remove the changes from yarn.lock and package.json files.

Yeah I did 😄

officeneerajsaini commented 1 month ago

The commit #1049 is also included in my pull request.

officeneerajsaini commented 1 month ago

@DhairyaMajmudar Please Review Code now.

DhairyaMajmudar commented 1 month ago

@officeneerajsaini pls. don't change the font colors

officeneerajsaini commented 1 month ago

@officeneerajsaini pls. don't change the font colors

In dark mode, some light colors look better than a dark-on-dark combination.

Consistent color:

https://github.com/user-attachments/assets/7fa172a0-1b4d-4dfa-bfe0-1c1f1c9dd76c

Some Lightness :

https://github.com/user-attachments/assets/99eb2397-6a14-4ee3-9945-266d136dfcf8

Should I go with the website theme color?

officeneerajsaini commented 1 month ago

Looks great, Thanks!!

However I do prefer the white color for the navbar fonts instead that light grey. Can we change that?

Yes, and what about the color for the active and hover effects?

officeneerajsaini commented 3 weeks ago

https://github.com/user-attachments/assets/9bb68b07-00d7-4f28-8a54-facde004797f

officeneerajsaini commented 2 weeks ago

@benjagm now take a look on code

benjagm commented 2 weeks ago

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?

officeneerajsaini commented 2 weeks ago

https://github.com/user-attachments/assets/238f50a2-bb47-4f62-ba21-bac1bfbd9542

benjagm commented 2 weeks ago

Hi everyone.

I checked the current version and found this behavior:

officeneerajsaini commented 2 weeks ago
  • Light style:

    • Style looks good
    • When navigating using links in the header, all styles to highlight the current section work with the exception of Tools. When in tools there is no style in the header highlight the current version.

Yes , Found that part too .

benjagm commented 2 days ago

@officeneerajsaini we are very close! Let's make the last change and push it. Great work so far. Thanks for your patience.