Skip to content
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 the tagindex issue #217

Merged
merged 7 commits into from
Mar 11, 2025
Merged

fix the tagindex issue #217

merged 7 commits into from
Mar 11, 2025

Conversation

Iamanujosh
Copy link
Contributor

@Iamanujosh Iamanujosh commented Mar 1, 2025

  • Issue: Issue on page /_tags/tagsindex.html #210
  • Fixed an issue where the logo was not switching properly in the "_tags" folder.
  • Added logic to detect if the page is inside the "_tags" directory and adjust logo paths accordingly.
  • Ensured that the logo updates correctly in both light and dark themes.
    image

@Iamanujosh Iamanujosh changed the title fix the tagindex issue fix the tagindex issue :https://github.com/numfocus/DISCOVER-Cookbook/issues/210 Mar 1, 2025
@Iamanujosh Iamanujosh changed the title fix the tagindex issue :https://github.com/numfocus/DISCOVER-Cookbook/issues/210 fix the tagindex issue Mar 1, 2025
@Iamanujosh
Copy link
Contributor Author

@aterrel Could you review this PR when you get a chance?

Copy link
Member

@aterrel aterrel left a comment

Choose a reason for hiding this comment

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

minor update and ready to go. Thanks for the patch

@aterrel aterrel added the 🔄 changes required need update before reviewing again label Mar 2, 2025
@AR21SM
Copy link
Contributor

AR21SM commented Mar 3, 2025

Hey @Iamanujosh, I have a small suggestion to make the code a bit cleaner while keeping the same functionality.

Instead of handling _tags/ conditions separately, you can use this approach:
if (logo.classList.contains('only-light')) {
logo.src = isTagsFolder ? '../_static/logo-light.png' : '_static/logo-light.png';
} else if (logo.classList.contains('only-dark')) {
logo.src = isTagsFolder ? '../_static/logo-dark.png' : '_static/logo-dark.png';
}

@Iamanujosh
Copy link
Contributor Author

Hey @aterrel ,

I wanted to check if you have any remarks on my contribution before we proceed further. If there are no concerns from your side, I’d love to refine it based on the suggestions given. However, if everything looks good, we can close the PR.

Let me know your thoughts so we can move forward accordingly.

CC: @AR21SM – Thanks for your input! I’m open to improvements, but I’d like to finalize the admin’s feedback first before making further changes.

@Iamanujosh
Copy link
Contributor Author

Hey @aterrel,

I have implemented the suggested changes by @AR21SM. Please review and let me know if any further modifications are needed. Looking forward to your feedback!

Thanks!

@aterrel
Copy link
Member

aterrel commented Mar 8, 2025

Looks a lot cleaner thanks @lamanujosh and @AR21SM

I want to play with a local build a bit and then can check it in. Been traveling so haven't had time to do it this past week.

@aterrel aterrel merged commit 95e9e55 into numfocus:main Mar 11, 2025
@aterrel
Copy link
Member

aterrel commented Mar 11, 2025

@all-contributors please add lamanujosh for code

Copy link
Contributor

@aterrel

Could not find the user lamanujosh on github.

@aterrel
Copy link
Member

aterrel commented Mar 11, 2025

@all-contributors please add Iamanujosh for code

Copy link
Contributor

@aterrel

I've put up a pull request to add @Iamanujosh! 🎉

@Iamanujosh
Copy link
Contributor Author

Thank you @aterrel

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🔄 changes required need update before reviewing again
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants