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

Set html lang attribute based upon locale #257

Merged
merged 1 commit into from Sep 13, 2017

Conversation

franek
Copy link
Contributor

@franek franek commented Sep 10, 2017

HTML lang attribute is always set to "en" for all themes. If descriptions are written in french and user agent (browser) is set in french, it may create some strange behaviors : enable browser automatic translations, speech synthesis will use english accent, ...

This PR tries to fix that. I am not a python expert so code I am publishing may surely be improved ;)

sigal/gallery.py Outdated

self.language = self.DEFAULT_LANGUAGE
if locale.getlocale():
self.language = locale.getlocale()[0][0:2]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Really don't like that. I would appreciate a better solution.

Copy link
Owner

Choose a reason for hiding this comment

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

This could simply be a setting. I would prefer this instead of relying on locale, users may want to customize it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@saimn I thought it was a good idea to relay on locale to avoid too many settings (for instance, dates are formatting using locale).

I will add a setting. Is language setting is enough precise or have you got a more relevant idea to name this new setting ?

Copy link
Owner

Choose a reason for hiding this comment

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

Yes it's once more setting, but I think it's good to be able to choose this language, same as for the locale setting.

Is language setting is enough precise

Not sure, it may give the feeling that the theme can be translated. I would probably go for a more specific name, maybe html_lang or html_language?

@saimn
Copy link
Owner

saimn commented Sep 12, 2017

Good idea to allow to customize this, but as said above it could be a setting instead.

@franek
Copy link
Contributor Author

franek commented Sep 13, 2017

@saimn I have updated my PR. Hope it will be ok for you ;)

@saimn saimn added this to the 1.4.0 milestone Sep 13, 2017
@saimn
Copy link
Owner

saimn commented Sep 13, 2017

Yes perfect, thanks!

@saimn saimn merged commit f6a8687 into saimn:master Sep 13, 2017
@franek
Copy link
Contributor Author

franek commented Sep 13, 2017

\o/

kontza pushed a commit to kontza/sigal that referenced this pull request Aug 28, 2020
Set html lang attribute based upon locale
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.

None yet

2 participants