feat(ui5-datepicker): implement ACC support #763

tsanislavgatev · 2019-09-03T08:37:13Z

Hi team, can you please review the PR.
Best Regards,
Tsani

ilhan007 · 2019-09-03T10:13:15Z

packages/main/src/Input.js

 		return this.showSuggestions ? "list" : undefined;
 	}

+	get roleAttribute() {
+		if (this._accInfo && this._accInfo.roleAttribute) {


Should be fine to return undefined, try
return this._accInfo && this._accInfo.roleAttribute;

Same for ariaOwns ariaExpanded

vladitasev

Looks great, please ask Eli to test it too

vladitasev · 2019-09-03T10:20:28Z

packages/main/src/DatePicker.hbs

@@ -46,6 +51,6 @@
 				@ui5-selectedDatesChange="{{_calendar.onSelectedDatesChange}}"
 		></ui5-calendar>
 	</ui5-popover>
-
+	<span id="{{_id}}-date" class="ui5-hidden-text">{{dateAriaDescriber}}</span>


Describer => Description

vladitasev · 2019-09-03T10:23:23Z

packages/main/src/DatePicker.js

+			"ariaDescribedBy": `${this._id}-date`,
+			"ariaHasPopup": "true",
+			"ariaAutoComplete": "none",
+			"roleAttribute": "combobox",


IMO just "role", they are all attributes

vladitasev · 2019-09-03T10:35:36Z

packages/main/src/i18n/messagebundle.properties

@@ -25,6 +25,12 @@ BUTTON_ARIA_TYPE_REJECT=Negative Action
 #XACT: ARIA announcement for the emphasized button
 BUTTON_ARIA_TYPE_EMPHASIZED=Emphasized

+#XACT: Aria information for the Date Picker


This is for the translation team, they don't really know what this means, so simply put a text such as "Date" IMO

vladitasev · 2019-09-03T10:37:40Z

packages/main/src/i18n/messagebundle.properties

@@ -25,6 +25,12 @@ BUTTON_ARIA_TYPE_REJECT=Negative Action
 #XACT: ARIA announcement for the emphasized button
 BUTTON_ARIA_TYPE_EMPHASIZED=Emphasized

+#XACT: Aria information for the Date Picker
+DATEPICKER_DATE_TYPE=Date


DATE_TYPE sounds inaccurate, maybe DATEPICKER_DATE_ACC_TEXT or something

vladitasev · 2019-09-03T10:38:51Z

packages/main/src/DatePicker.js

+			"ariaAutoComplete": "none",
+			"roleAttribute": "combobox",
+			"ariaOwns": `${this._id}-popover`,
+			"ariaExpanded": this.isOpen(),


What happens when the value of this.isOpen() is changed? Does something invalidate so that the component can be invalidated?

When the picker gets opened or closed this. _isPickerOpen is mutated, which invalidates the ui5-datepicker, and this.isOpen() will return either true, or false when calculated for the HBS template upon rendering. (If that was the question)

ilhan007 · 2019-09-04T06:06:33Z

packages/main/src/Input.js

 		return `${this.suggestionsTextId} ${this.valueStateTextId}`.trim();
 	}

 	get ariaHasPopup() {
+		if (this._accInfo && this._accInfo.ariaHasPopup) {


You can do one check less, as undefined is fine:

if (this._accInfo) {
return this._accInfo.ariaHasPopup;
}

ilhan007 · 2019-09-04T06:07:17Z

packages/main/src/Input.js

 		return this.showSuggestions ? "true" : undefined;
 	}

 	get ariaAutoComplete() {
+		if (this._accInfo && this._accInfo.ariaAutoComplete) {


Same as above, you can do one check less, as undefined is fine:

if (this._accInfo) {
return this._accInfo. ariaAutoComplete;
}

vladitasev · 2019-09-05T11:45:02Z

packages/main/src/Input.js

+		return {
+			"wrapperAccInfo": {
+			},
+			"inputAccInfo": {


AccInfo is already in the name of the main object, so "input" and "wrapper" are enough here
accInfo.input.role f.e.

vladitasev · 2019-09-05T12:16:18Z

packages/main/src/Input.js

+			"inputAccInfo": {
+				"ariaDescribedBy": this._inputAccInfo ? `${this.suggestionsTextId} ${this.valueStateTextId} ${this._inputAccInfo.ariaDescribedBy}`.trim() : `${this.suggestionsTextId} ${this.valueStateTextId}`.trim(),
+				"ariaInvalid": this.valueState === ValueState.Error ? "true" : undefined,
+				"ariaHasPopup": this._inputAccInfo ? this._inputAccInfo.ariaHasPopup : () => { return this.showSuggestions ? "true" : undefined; },


IMO better to extract this as a const above
same for the other such lines, seems a bit too complex to read

elenastoyanovaa · 2019-09-10T06:36:02Z

packages/main/src/DatePicker.hbs

@@ -11,16 +11,21 @@
 			?disabled="{{disabled}}"
 			?readonly="{{readonly}}"
 			value-state="{{valueState}}"
+			aria-autocomplete="none"


remove the aria stuff from the ui5-input

elenastoyanovaa · 2019-09-10T06:36:59Z

packages/main/src/DatePicker.hbs

@@ -46,6 +51,6 @@
 				@ui5-selectedDatesChange="{{_calendar.onSelectedDatesChange}}"
 		></ui5-calendar>
 	</ui5-popover>
-
+	<span id="{{_id}}-date" class="ui5-hidden-text">{{dateAriaDescription}}</span>


this is not being read out when it is outside the native input DOM. We should add it in the same shadow root as the native input element

elenastoyanovaa

Everything works as expected and it is being read out correctly. Just look at the comments, there are a few minor stuff which should be changed

tsanislavgatev added 17 commits August 26, 2019 14:05

feat: implement create control script

767525a

fix: create control script pr notes fix

2f2ba11

add console error on empty string for a name of component

f722bb6

solve conflict

1357c13

master update

849e594

removed hungarian notation

bd9bf48

removed old component creation snippet file

4b82ba5

feat(ui5-tabcontainer): acc update

f77bfb6

merge conflict solve

8d26917

merge conflict tags solve

a9235f7

feat(ui5-tabcontainer): update ACC of header and content

fdd2118

feat(ui5-tabcontainer): update ACC of header and content

af2422c

Merge branch 'master' of https://github.com/tsanislavgatev/ui5-webcom…

23be11a

…ponents

Merge branch 'master' of https://github.com/tsanislavgatev/ui5-webcom…

4850ce6

…ponents

Merge branch 'master' of https://github.com/tsanislavgatev/ui5-webcom…

3c71085

…ponents

feat:(ui5-datepicker): update ACC of the component and input included

067dbaa

Merge remote-tracking branch 'upstream/master'

9d4f2db

ilhan007 reviewed Sep 3, 2019

View reviewed changes

vladitasev requested changes Sep 3, 2019

View reviewed changes

Updating with comments from PR

874d752

vladitasev requested a review from elenastoyanovaa September 3, 2019 11:27

ilhan007 changed the title ~~ACC implementation of DatePicker component~~ feat(ui5-datepicker): implement ACC support Sep 4, 2019

ilhan007 reviewed Sep 4, 2019

View reviewed changes

tsanislavgatev added 2 commits September 4, 2019 13:30

Update with notes from PR

6d1dc16

Improve of get methods for acc in input

5ed53a2

vladitasev reviewed Sep 5, 2019

View reviewed changes

update with comments from pr

70b5104

elenastoyanovaa reviewed Sep 10, 2019

View reviewed changes

elenastoyanovaa requested changes Sep 10, 2019

View reviewed changes

changing the place of input parent information

d6598da

elenastoyanovaa previously approved these changes Sep 10, 2019

View reviewed changes

update escaping information repeating

0c842f7

tsanislavgatev dismissed elenastoyanovaa’s stale review via 0c842f7 September 10, 2019 11:29

vladitasev approved these changes Sep 10, 2019

View reviewed changes

vladitasev merged commit 188627e into SAP:master Sep 10, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(ui5-datepicker): implement ACC support #763

feat(ui5-datepicker): implement ACC support #763

tsanislavgatev commented Sep 3, 2019

ilhan007 Sep 3, 2019

vladitasev left a comment

vladitasev Sep 3, 2019

vladitasev Sep 3, 2019

vladitasev Sep 3, 2019

vladitasev Sep 3, 2019

vladitasev Sep 3, 2019

ilhan007 Sep 4, 2019

ilhan007 Sep 4, 2019

ilhan007 Sep 4, 2019

vladitasev Sep 5, 2019

vladitasev Sep 5, 2019

elenastoyanovaa Sep 10, 2019

elenastoyanovaa Sep 10, 2019

elenastoyanovaa left a comment

feat(ui5-datepicker): implement ACC support #763

feat(ui5-datepicker): implement ACC support #763

Conversation

tsanislavgatev commented Sep 3, 2019

Choose a reason for hiding this comment

vladitasev left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

elenastoyanovaa left a comment

Choose a reason for hiding this comment