yoast-components icon indicating copy to clipboard operation
yoast-components copied to clipboard

Snippet editor: improve aria-labelledby and aria-describedby usage (also Failed prop type warning)

Open afercia opened this issue 7 years ago • 3 comments

Noticed while CR'ing #600 and #545

aria-labelledby

In https://github.com/Yoast/yoast-components/pull/545/files#r196005254 the aria-labelledby value of the Title and Description fields were simplified and they're now just a number. While, if I remember correctly, ID values that start with a number are perfectly valid in HTML5 (they were not in HTML4), I'd recommend to consider to add a prefix or something to better qualify these values and avoid potential conflicts with other IDs.

Also, there's a bit of inconsistency with the slug field which still uses a prefix + number.

screen shot 2018-06-18 at 10 21 37

screen shot 2018-06-18 at 11 01 49

aria-describedby

This aria property is used to associate a field to the description provided via the "visual placeholder" we're using on the description field:

screen shot 2018-06-18 at 10 46 30

That's good, however also the Title field has an aria-describedby attribute which points to nothing, as there's no description for the title field. aria-describedby should always reference other existing elements.

afercia avatar Jun 18 '18 09:06 afercia

Also, not sure how the text passed via the default prop can be translatable: descriptionEditorFieldPlaceholder: "Modify your meta description by editing it right here"

afercia avatar Jul 12 '18 09:07 afercia

For the aria-describedby still rendered even when there's no placeholder, see https://github.com/facebook/draft-js/issues/1739 and https://github.com/facebook/draft-js/pull/1741 still pending feedback from April 25, 2018.

afercia avatar Jul 12 '18 09:07 afercia

Worth noting there's also a warning Warning: Failed prop type: Invalid prop ariaLabelledByof typenumbersupplied toReplacementVariableEditorStandalone, expected string.

afercia avatar Nov 01 '18 13:11 afercia