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

Use flag for language selector icon #2062

Merged
merged 1 commit into from Mar 1, 2023

Conversation

jonaharagon
Copy link
Member

@jonaharagon jonaharagon commented Mar 1, 2023

Screenshot 2023-03-01 at 11 11 01@2x

@netlify
Copy link

netlify bot commented Mar 1, 2023

Deploy Preview for privacyguides ready!

Name Link
🔨 Latest commit e336699
🔍 Latest deploy log https://app.netlify.com/sites/privacyguides/deploys/63ffa1dc885e2d0008474b0a
😎 Deploy Preview https://deploy-preview-2062--privacyguides.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 site settings.

@kamilkrzyskow
Copy link

kamilkrzyskow commented Mar 1, 2023

The hook replaces on the fly the mkdocs-material site-packages raw header.html partial https://github.com/squidfunk/mkdocs-material/blob/master/material/partials/header.html.
You have your own https://github.com/privacyguides/privacyguides.org/blob/main/theme/partials/header.html partial which "overwrites" the changes made by the hook.
Also I worked with the Public version, I don't know if Insiders made any changes to the partial, but I doubt it, because otherwise the hook would log a warning.

jonaharagon added a commit that referenced this pull request Mar 1, 2023
Revert "Use flag for language selector icon"

This reverts commit ea5c2ff.

Use flag for language selector icon (#2062)
@jonaharagon jonaharagon force-pushed the jonaharagon/use-flag-for-lang-selector branch from ea5c2ff to 67f08d2 Compare March 1, 2023 17:06
@jonaharagon
Copy link
Member Author

@kamilkrzyskow yeah I was testing without that override and it still didn't work, but it seems to be related to #2063 in that case, which is yet another entirely unrelated bug :)

Anyways I can just do this a different way with jinja formatting it turns out, so I'll just go with that for now. Ideally theme/partials/header.html could be replaced with dynamic overrides like in squidfunk/mkdocs-material#5060 in the long-term, but at the moment I'm not good enough at Python to figure that out.

jonaharagon added a commit that referenced this pull request Mar 1, 2023
@jonaharagon jonaharagon force-pushed the jonaharagon/use-flag-for-lang-selector branch from 67f08d2 to 39f484a Compare March 1, 2023 17:10
@jonaharagon jonaharagon marked this pull request as ready for review March 1, 2023 17:10
@kamilkrzyskow
Copy link

kamilkrzyskow commented Mar 1, 2023

Your solution with the additional icon in the alternate is very clean. I really like it.


I mentioned your partial override, because the commit didn't remove it.
As for the #2063, you're most likely correct, as the on_post_page event includes the changes made by the hook.
But why would it crash for an .svg external resource with some Empty Document error 🤔

I tested the hook locally and after removing your custom partial it works fine 😮‍💨 (of course this is the old Public version without the privacy plugin).

flags

@jonaharagon jonaharagon force-pushed the jonaharagon/use-flag-for-lang-selector branch from 39f484a to e336699 Compare March 1, 2023 19:04
@jonaharagon jonaharagon merged commit e336699 into main Mar 1, 2023
@jonaharagon jonaharagon deleted the jonaharagon/use-flag-for-lang-selector branch March 1, 2023 19:08
@privacyguides-bot
Copy link
Collaborator

This pull request has been mentioned on Privacy Guides. There might be relevant details there:

https://discuss.privacyguides.net/t/v3-3/11950/1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants