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.

Sign up for GitHub

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

Jump to bottom

add support for configuring lang attribute in default template #558

Closed
1 of 5 tasks
thescientist13 opened this issue Apr 16, 2021 · 3 comments · Fixed by #892
Closed
1 of 5 tasks

add support for configuring lang attribute in default template #558

thescientist13 opened this issue Apr 16, 2021 · 3 comments · Fixed by #892
Labels
feature New feature or request good first issue Good for newcomers question Further information is requested
Milestone
1.0

Comments

@thescientist13
Copy link
Member

Type of Change

  • New Feature Request
  • Documentation / Website
  • Improvement / Suggestion
  • Bug
  • Other (please clarify below)

Summary

As part of the PR for #512, the question came up regarding default app templates and what the default HTML tags should, and in particular the lang attribute.

<html lang="en" prefix="og:http://ogp.me/ns#">

It is a required attribute, but it's also meant to be contextual to the language content of the project.

Details

My first thought would be that user's could provide a lang config property, and that we could swap out that part for them.

// greenwood.config.js
module.exports = {
  lang: "xx"
}

Then we would return the default template with their lang

<html lang="xx" prefix="og:http://ogp.me/ns#">

Open to other options, but I think it would be important to make this easy, but not also not sure how often someone would want to configure lang but not also be using their own app template? 🤔
@thescientist13 thescientist13 added enhancement Improve something existing (e.g. no docs, new APIs, etc) good first issue Good for newcomers question Further information is requested labels Apr 16, 2021
@thescientist13 thescientist13 added this to the 1.0 milestone Apr 16, 2021
@thescientist13 thescientist13 mentioned this issue Apr 16, 2021
Merged
2 tasks
@mknee40
Copy link

mknee40 commented May 8, 2021

Wouldn't just manually changing the lang attribute have the same effort as adding it in the configuration file? Im not sure how many templates you would be using, but even if you had 10 templates, this is still not going to take long to update.
Does the language configuration affect anything other than the value in the html template, for example, any translation components?

@thescientist13
Copy link
Member Author

thescientist13 commented May 8, 2021 •
edited
Loading

Yeah, that's definitely a good point regarding just writing it all in HTML as opposed to wrapping it up all in JavaScript somewhere. For example, the same could be said of being able to pass <meta> tags programmatically (an earlier design decision / solution pre v0.10.0) where as to your point, it would probably be just as easy now (arguably easier) to just write the plain HTML right in your own code.

It's definitely a question I have had in the back of my mind.

Plus, this would only account for our default app-template.html anyway so, it's a good question to say: how much work should be put into a potentially minimally used feature? (potentially). 🤔

@thescientist13 thescientist13 added feature New feature or request and removed enhancement Improve something existing (e.g. no docs, new APIs, etc) labels Feb 9, 2022
@thescientist13
Copy link
Member Author

Not sure if this is worth it, along the lines of #851 ? Probably just recommend users set it themselves, or if it does need to be configurable, then that use case is likely in the scope of i18n which is a much larger conversation to be had.

@thescientist13 thescientist13 mentioned this issue Feb 10, 2022
Closed
@thescientist13 thescientist13 mentioned this issue Feb 17, 2022
Merged
6 tasks
@thescientist13 thescientist13 linked a pull request Feb 17, 2022 that will close this issue
Enhancement/issue 881 favor content over configuration eg meta #892
Merged
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
feature New feature or request good first issue Good for newcomers question Further information is requested
Projects
None yet
Milestone
1.0
Development

Successfully merging a pull request may close this issue.

Enhancement/issue 881 favor content over configuration eg meta ProjectEvergreen/greenwood
2 participants
@thescientist13 @mknee40