feat: allow users to add arialabel to text input #5560
Conversation
|
One of the public type files has been updated, plase make sure there are no backwards incompatible changes done in the PR. |
| @@ -235,6 +235,7 @@ export namespace Ace { | |||
| relativeLineNumbers: boolean; | |||
| enableMultiselect: boolean; | |||
| enableKeyboardAccessibility: boolean; | |||
| textInputAriaLabel: string; | |||
There was a problem hiding this comment.
So I guess nothing is optional here? Although everything is
There was a problem hiding this comment.
yeah all of the options are marked as mandatory, users using this type will likely have to use Partial<EditorOptions>.
There was a problem hiding this comment.
I see we have it in https://github.com/ajaxorg/ace/blob/master/ace.d.ts#L1104
src/keyboard/textinput.js
Outdated
| @@ -90,7 +90,8 @@ TextInput= function(parentNode, host) { | |||
| text.setAttribute("aria-roledescription", nls("text-input.aria-roledescription", "editor")); | |||
| if(host.session) { | |||
| var row = host.session.selection.cursor.row; | |||
| text.setAttribute("aria-label", nls("text-input.aria-label", "Cursor at row $0", [row + 1])); | |||
| var arialLabel = `${host.$textInputAriaLabel}, ${nls("text-input.aria-label", "Cursor at row $0", [row + 1])}`; | |||
There was a problem hiding this comment.
should we hide "," if there is textInputAriaLabel is not there? Basically to cater for the initial value.
There was a problem hiding this comment.
yeah that's fair, changed
src/keyboard/textinput_test.js
Outdated
| @@ -753,6 +753,18 @@ module.exports = { | |||
| assert.equal(editor.getValue(), ""); | |||
| sendEvent("input", {key: {inputType: "historyRedo"}}); | |||
| assert.equal(editor.getValue(), "x"); | |||
| }, | |||
|
|
|||
| "test: text input aria label": function() { | |||
There was a problem hiding this comment.
Can we also add same test but without textInputAriaLabel being set.
|
One of the public type files has been updated, plase make sure there are no backwards incompatible changes done in the PR. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #5560 +/- ##
=======================================
Coverage 86.66% 86.67%
=======================================
Files 593 593
Lines 42920 42940 +20
Branches 7125 7126 +1
=======================================
+ Hits 37198 37218 +20
Misses 5722 5722
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Issue #, if available:
Description of changes: Currently, the aria-label of the text input says
Cursor at row x, some users might want to add context about their editor to this aria label. This change adds an option,textInputAriaLabel, which is used to create and aria-label astextInputAriaLabel, Cursor at row x.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: