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

Section field uses h1 #453

Closed
NicoHood opened this issue Nov 29, 2020 · 4 comments
Closed

Section field uses h1 #453

NicoHood opened this issue Nov 29, 2020 · 4 comments

Comments

@NicoHood
Copy link
Contributor

NicoHood commented Nov 29, 2020

<h1 class="{% if not field.underline %}no_underline{% endif %}">{{ field.title|t }}</h1>

The section field uses h1 by default. This is a really bad idea, as a page should not have more then one h1. Also most likely you will have a different h1 for the page, maybe a h2 after and a h3 inside the section. The reason why this is important is search engine optimization (SEO).

I know this template can be overwritten in the theme, but I'd like to fix it upstream, if possible. My suggestion:

  • Change the new default to h3
  • Introduce an optional field setting title_level which can be set to h4 etc.
<{{ field.title_level|default('h3') }} class="{% if not field.underline %}no_underline{% endif %}">{{ field.title|t }}</{{ field.title_level|default('h3') }}>

The change only affects frontend from, not the admin. The admin also uses h1, but does not need a search engine optimization, so it is less important.

@rhukster
Copy link
Member

rhukster commented Dec 1, 2020

This is probably a better idea, however, this field is primarily used in Admin, where h1 was already styled the way we wanted. Also changing it now would mean a breaking change. So something we'll handle better next time.

As a workaround you can override the section.html.twig and change your end.

@rhukster rhukster closed this as completed Dec 1, 2020
@NicoHood
Copy link
Contributor Author

NicoHood commented Dec 1, 2020

The admin template is located here, will this be pulled from the forms plugin?:
https://github.com/getgrav/grav-plugin-admin/blob/develop/themes/grav/templates/forms/fields/section/section.html.twig

What about this? It is 100% backwards compatible, but still fixes the issue:

<{{ field.title_level|default('h1') }} class="{% if not field.underline %}no_underline{% endif %}">{{ field.title|t }}</{{ field.title_level|default('h1') }}>

@mahagr mahagr reopened this Dec 2, 2020
@mahagr
Copy link
Member

mahagr commented Dec 2, 2020

@rhukster ?

@rhukster
Copy link
Member

rhukster commented Dec 2, 2020

I'm ok with this if you create a PR.

Cheers!

NicoHood added a commit to NicoHood/grav-plugin-form that referenced this issue Dec 2, 2020
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

No branches or pull requests

3 participants