Skip to content

Conversation

@ldez
Copy link
Contributor

@ldez ldez commented Aug 16, 2025

The PR is the same as #499 but updated to Tailwind 4 and with some minor changes.

To answer the question about 12/24. (https://github.com/imfing/hextra/pull/499/files#r1874791928)

12 is the height of the icons/svg inside the theme toggle:

24 is the height of the icons/svg inside the navbar:

Closes #499
Fixes #343

@netlify
Copy link

netlify bot commented Aug 16, 2025

Deploy Preview for hugo-hextra ready!

Name Link
🔨 Latest commit 5e3cabf
🔍 Latest deploy log https://app.netlify.com/projects/hugo-hextra/deploys/68a0a5d227901300081636e7
😎 Deploy Preview https://deploy-preview-759--hugo-hextra.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@ldez ldez changed the title feat(theme-toggle): toggle theme inside navbar feat(navbar): toggle theme inside navbar Aug 16, 2025
@ldez
Copy link
Contributor Author

ldez commented Aug 16, 2025

I changed the implementation to use parameters for icon and text height instead of hardcoding the value based on .navbar.

The two approaches work; if you prefer one over the other, I can adjust the implementation accordingly.

@ldez ldez force-pushed the feat/theme-toggle-navbar branch from d489f4a to 7356ae5 Compare August 16, 2025 10:00
Copy link
Owner

@imfing imfing left a comment

Choose a reason for hiding this comment

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

Thanks! It looks good overall, with just a few small suggestions

Copy link
Owner

@imfing imfing left a comment

Choose a reason for hiding this comment

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

Image Image

tweaked the classes to make the styles consistent with other icons in the navbar

Co-authored-by: Xin <[email protected]>
Copy link
Owner

@imfing imfing left a comment

Choose a reason for hiding this comment

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

implementation looks good, let's add it to the documentation

@ldez
Copy link
Contributor Author

ldez commented Aug 16, 2025

I added the documentation, but I don't speak Chinese, Japanese, or Farsi.

I can use tools to translate it, but I cannot be sure of the translations.

@imfing
Copy link
Owner

imfing commented Aug 16, 2025

I added the documentation, but I'm not speaking Chinese, Japanese, or Farsi.

I can use tools to translate it, but I cannot be sure of the translations.

thanks, it's totally fine. The translations were done by LLMs so I'm not sure either 😅

@imfing imfing merged commit 6613f94 into imfing:main Aug 16, 2025
4 checks passed
@ldez ldez deleted the feat/theme-toggle-navbar branch August 16, 2025 15:46
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.

Add light/dark theme switch to navigation

2 participants