search

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
730 stars 736 forks source link

[Page] Confusing page titles should be clarified #160

Closed kwin closed 4 years ago

kwin commented 6 years ago

Currently in the page properties dialog there is a dedicated input field for property "pageTitle" (inherited from wcm/foundation/components/basicpage/v1/basicpage)

In the model this jcr property is not considered (https://github.com/Adobe-Marketing-Cloud/aem-core-wcm-components/blob/da7b1c440d1824972671c500bd272b710716e0ab/bundles/core/src/main/java/com/adobe/cq/wcm/core/components/internal/models/v1/PageImpl.java#L112) as only Page.getTitle() (https://helpx.adobe.com/experience-manager/6-3/sites/developing/using/reference-materials/javadoc/com/day/cq/wcm/api/Page.html#getTitle()) is being used but not Page.getPageTitle() (https://helpx.adobe.com/experience-manager/6-3/sites/developing/using/reference-materials/javadoc/com/day/cq/wcm/api/Page.html#getPageTitle()) therefore the head->title will never switch to the value maintained as Page Title in the page properties.

bpauli commented 6 years ago

@kwin Thank you for the hands up. I just checked the "history" of this property and it looks like that this property was never used in the page component directly (even not in the old foundation page component). The only reference is in the Title component (for example in [1]) when no jcr:title is set.

I think we have to update the documentation to make clear where this property is used for or we should implement an adional fallback in the Page model when no jcr:title is provided.

cc @gabrielwalt

[1] https://github.com/Adobe-Marketing-Cloud/aem-core-wcm-components/blob/master/bundles/core/src/main/java/com/adobe/cq/wcm/core/components/internal/models/v1/TitleImpl.java#L69

kwin commented 6 years ago

The pagetitle is already being used in https://github.com/Adobe-Marketing-Cloud/aem-core-wcm-components/blob/da7b1c440d1824972671c500bd272b710716e0ab/bundles/core/src/main/java/com/adobe/cq/wcm/core/components/internal/models/v1/PageListItemImpl.java#L60

bpauli commented 6 years ago

Good catch.

@gabrielwalt, the pageTitle property is currently used by the List [1] and Title [2] component. Should we also update the page model implementation to have the same fallback for https://github.com/Adobe-Marketing-Cloud/aem-core-wcm-components/blob/da7b1c440d1824972671c500bd272b710716e0ab/bundles/core/src/main/java/com/adobe/cq/wcm/core/components/models/Page.java#L225 or would this be a breaking change?

[1] https://github.com/Adobe-Marketing-Cloud/aem-core-wcm-components/blob/da7b1c440d1824972671c500bd272b710716e0ab/bundles/core/src/main/java/com/adobe/cq/wcm/core/components/internal/models/v1/PageListItemImpl.java#L60

[2] https://github.com/Adobe-Marketing-Cloud/aem-core-wcm-components/blob/da7b1c440d1824972671c500bd272b710716e0ab/bundles/core/src/main/java/com/adobe/cq/wcm/core/components/internal/models/v1/TitleImpl.java#L69

gabrielwalt commented 6 years ago

The initial idea was that the "page title" is displayed in the page (for instance by an H1), which has to be different from the top title (in the HEAD) when that one gets filled with SEO keywords. Beyond this being confusing, it also requires all components to implement that they take the page title (when it's not empty) instead of the title, which requires something special to be done by each component.

Instead, I'd rather change the page to have an SEO title field to modify the top title (in the HEAD) when needed. The regular title would then be used by all components (list, teaser, search results, etc.), except for navigation components that use the navigation title (when it's not empty).

If we agree that this would be a desired improvement, it would be a breaking change that we'd do along with other breaking changes that we might want to do on the page component (like cleaning up the dialogs, adding a page image property, and making the available page properties configurable from the design dialogs).

(CQ-4247353)

kwin commented 6 years ago

Ok, I understand. But then clearly the list component should not leverage that property either in https://github.com/Adobe-Marketing-Cloud/aem-core-wcm-components/blob/da7b1c440d1824972671c500bd272b710716e0ab/bundles/core/src/main/java/com/adobe/cq/wcm/core/components/internal/models/v1/PageListItemImpl.java#L60 and for sure the description in the page properties should be clarified.

richardhand commented 4 years ago

This was fixed in https://github.com/adobe/aem-core-wcm-components/pull/834, is merged to master and will be available in the upcoming 2.8.0 release.