adobe / aem-core-wcm-components

Standardized components to build websites with AEM.
https://docs.adobe.com/content/help/en/experience-manager-core-components/using/introduction.html
Apache License 2.0
740 stars 747 forks source link

Teaser component uses hardcoded h2 tag - semantically incorrect #925

Closed henrykuijpers closed 4 years ago

henrykuijpers commented 4 years ago

https://github.com/adobe/aem-core-wcm-components/blob/c69fdc63fc0f2fb122d65eb4091d9bef872e09fb/content/src/content/jcr_root/apps/core/wcm/components/teaser/v1/teaser/title.html

The teaser component renders a h2 tag for the title, always. This is semantically incorrect in a lot of cases. The h1/h2/h3/h4/... should be configurable, in the same way this is done with the title-component.

Bug Report

Current Behavior Teaser component always renders a h2.

Expected behavior/code Teaser component should have an option to specify the title-depth to render.

Possible Solution Include the functionality of the title-component also in the teaser-component.

godanny86 commented 4 years ago

@henrykuijpers this configuration is available via the Teaser's policy configuration: https://docs.adobe.com/content/help/en/experience-manager-core-components/using/components/teaser.html#teaser-tab. Although this means that all teasers on the page will have the same element has dictated by the policy...

henrykuijpers commented 4 years ago

@godanny86 @vladbailescu Oh my god. HTL is so confusing at times, I completely overlooked the data-sly-element attribute on that h2.

So now the next few questions arise:

  1. What is your idea on semantics with regards to the teaser component? I.e. we have teasers that should represent a h1 and we have teasers that should represent a h2. As I understand, it's possible to only configure one content policy for a teaser on a page (if you let the content author build his/her page completely, without defining regions/layout containers inside the template);
  2. Is it an idea to put the heading size option also in the normal dialog, instead of only in the design dialog, so that authors can choose their heading size?
  3. We're currently using the style system to make the teasers look different, should this have been components instead of style options? (So that we can specify the title sizes in their content policies.)
nathanmckean commented 4 years ago

The real issue is that the content editor can't select the heading tag on the title. It is a single selection. It should be selectable like with the title component.

Mentioned in better detail here: https://github.com/adobe/aem-core-wcm-components/issues/684

gabrielwalt commented 4 years ago

+1 to improve this, as I just commented on #684. Closing this as a duplicate of #684. Your contributions as PR would be very welcome to improve this!