fix: improve aria attributes of popup elements #5739
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #5739 +/- ##
=======================================
Coverage 87.06% 87.06%
=======================================
Files 598 598
Lines 43719 43722 +3
Branches 7164 7164
=======================================
+ Hits 38064 38067 +3
Misses 5655 5655
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
src/autocomplete/popup.js
Outdated
| var t = popup.renderer.$textLayer; | ||
| for (var row = t.config.firstRow, l = t.config.lastRow; row <= l; row++) { | ||
| const popupRowElement = /** @type {HTMLElement|null} */(t.element.childNodes[row - t.config.firstRow]); | ||
| const ariaLabel = `${popup.getData(row).caption || popup.getData(row).value}${popup.getData(row).meta ? `, ${popup.getData(row).meta}` : ''}`; |
There was a problem hiding this comment.
| const ariaLabel = `${popup.getData(row).caption || popup.getData(row).value}${popup.getData(row).meta ? `, ${popup.getData(row).meta}` : ''}`; | |
| const rowData = popup.getData(row); | |
| const ariaLabel = `${rowData.caption || rowData.value}${rowData.meta ? `, ${rowData.meta}` : ''}`; |
src/layer/text.js
Outdated
| @@ -424,7 +424,15 @@ class Text { | |||
| span.className = classes; | |||
| span.appendChild(valueFragment); | |||
|
|
|||
| parent.appendChild(span); | |||
| if (token.type === "completion-highlight") { | |||
There was a problem hiding this comment.
this does not belong in text.js which is the editor renderer, and should not contain code specific to the popup. What are you trying to accomplish with this code?
There was a problem hiding this comment.
this code aims to improve accessibility, the reason being that screen reader users will not be aware of the presence of the highlighted text otherwise. is there any other place in the code you would recommend me to add this logic to?
There was a problem hiding this comment.
changed to role=mark instead of mark element, and set it from the afterRender hook where other popup aria attributes are handled
Issue #, if available:
Description of changes:
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
Pull Request Checklist:
ace.d.ts) and its references: