Skip to content

Commit

Permalink
fix(ui5-input): aria-required attribute removed (#2552)
Browse files Browse the repository at this point in the history
Some of the input-based components have had both "required" and "aria-required" attributes rendered to the inner input when the required property of the component is set to true which is not necessary.
Now, only "aria-required" attribute is rendered, since it is more appropriate in order to avoid "Invalid entry" to be read out when the component is accessed and has no value, which is the expected behaviour when "required" attribute is set, but is a little bit confusing for the users.
  • Loading branch information
niyap authored Dec 9, 2020
1 parent 766bcc0 commit 7456ab5
Show file tree
Hide file tree
Showing 12 changed files with 22 additions and 10 deletions.
2 changes: 1 addition & 1 deletion packages/main/src/ComboBox.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
placeholder="{{placeholder}}"
?disabled={{disabled}}
?readonly={{readonly}}
?required={{required}}
value-state="{{valueState}}"
@input="{{_input}}"
@change="{{_inputChange}}"
Expand All @@ -23,6 +22,7 @@
aria-autocomplete="both"
aria-describedby="{{valueStateTextId}}"
aria-label="{{ariaLabelText}}"
aria-required="{{required}}"
/>

{{#if icon}}
Expand Down
1 change: 1 addition & 0 deletions packages/main/src/DatePicker.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
type="{{type}}"
value="{{value}}"
?disabled="{{disabled}}"
?required="{{required}}"
?readonly="{{readonly}}"
value-state="{{valueState}}"
@ui5-change="{{_handleInputChange}}"
Expand Down
1 change: 0 additions & 1 deletion packages/main/src/DatePicker.js
Original file line number Diff line number Diff line change
Expand Up @@ -823,7 +823,6 @@ class DatePicker extends UI5Element {
"ariaOwns": `${this._id}-responsive-popover`,
"ariaExpanded": this.isOpen(),
"ariaDescription": this.dateAriaDescription,
"ariaRequired": this.required,
"ariaLabel": getEffectiveAriaLabelText(this),
};
}
Expand Down
3 changes: 1 addition & 2 deletions packages/main/src/Input.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
?inner-input-with-icon="{{icon.length}}"
?disabled="{{disabled}}"
?readonly="{{_readonly}}"
?required="{{required}}"
.value="{{value}}"
placeholder="{{placeholder}}"
maxlength="{{maxlength}}"
Expand All @@ -25,7 +24,7 @@
aria-autocomplete="{{accInfo.input.ariaAutoComplete}}"
aria-expanded="{{accInfo.input.ariaExpanded}}"
aria-label="{{accInfo.input.ariaLabel}}"
aria-required="{{accInfo.input.ariaRequired}}"
aria-required="{{required}}"
@input="{{_handleInput}}"
@change="{{_handleChange}}"
@keydown="{{_onkeydown}}"
Expand Down
1 change: 0 additions & 1 deletion packages/main/src/Input.js
Original file line number Diff line number Diff line change
Expand Up @@ -1067,7 +1067,6 @@ class Input extends UI5Element {
"ariaExpanded": this._inputAccInfo && this._inputAccInfo.ariaExpanded,
"ariaDescription": this._inputAccInfo && this._inputAccInfo.ariaDescription,
"ariaLabel": (this._inputAccInfo && this._inputAccInfo.ariaLabel) || getEffectiveAriaLabelText(this),
"ariaRequired": (this._inputAccInfo && this._inputAccInfo.ariaRequired) || this.required,
},
};
}
Expand Down
2 changes: 1 addition & 1 deletion packages/main/src/MultiComboBox.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@
placeholder="{{placeholder}}"
?disabled={{disabled}}
?readonly={{readonly}}
?required={{required}}
value-state="{{valueState}}"
@input="{{_inputLiveChange}}"
@change={{_inputChange}}
Expand All @@ -53,6 +52,7 @@
aria-autocomplete="both"
aria-labelledby="{{_id}}-hiddenText-nMore"
aria-describedby="{{valueStateTextId}}"
aria-required="{{required}}"
/>

{{#if icon}}
Expand Down
1 change: 0 additions & 1 deletion packages/main/src/MultiComboBoxPopover.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@
.value="{{value}}"
inner-input
placeholder="{{placeholder}}"
?required={{required}}
value-state="{{valueState}}"
@input="{{_inputLiveChange}}"
@change={{_inputChange}}
Expand Down
2 changes: 1 addition & 1 deletion packages/main/src/TextArea.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,9 @@
placeholder="{{placeholder}}"
?disabled="{{disabled}}"
?readonly="{{readonly}}"
?required="{{required}}"
aria-label="{{ariaLabelText}}"
aria-describedby="{{ariaDescribedBy}}"
aria-required="{{required}}"
maxlength="{{_exceededTextProps.calcedMaxLength}}"
.value="{{value}}"
@input="{{_oninput}}"
Expand Down
1 change: 1 addition & 0 deletions packages/main/test/pages/DatePicker_test_page.html
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
<ui5-date-picker id="dp8" value="Jan 6, 2015"></ui5-date-picker>
<ui5-date-picker id="dp9" value="today"></ui5-date-picker>
<ui5-date-picker id="dp10" disabled></ui5-date-picker>
<ui5-date-picker id='dp-required' required></ui5-date-picker>
<ui5-date-picker id="dp11"></ui5-date-picker>
<ui5-date-picker id="dp13"></ui5-date-picker>
<ui5-date-picker id="dp16" format-pattern="long"></ui5-date-picker>
Expand Down
12 changes: 12 additions & 0 deletions packages/main/test/specs/DatePicker.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,18 @@ describe("Date Picker Tests", () => {
assert.ok(!datepicker.hasIcon(), "icon is not displayed");
});

it("required", () => {
datepicker.id = "#dp-required";

assert.ok(datepicker.input.getProperty("required"), "input has required set");
assert.strictEqual(datepicker.innerInput.getAttribute("aria-required"), "true", "Aria-required attribute is set correctly.");

datepicker.root.removeAttribute("required");

assert.notOk(datepicker.input.getProperty("required"), "required property is not set");
assert.strictEqual(datepicker.innerInput.getAttribute("aria-required"), "false", "Aria-required attribute is set correctly.");
});

it("placeholder", () => {
datepicker.root.setAttribute("placeholder", "test placeholder");

Expand Down
3 changes: 2 additions & 1 deletion packages/main/test/specs/Input.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@ describe("Attributes propagation", () => {
});

it("Required attribute is propagated properly", () => {
assert.ok(browser.$("#input-required").shadow$(".ui5-input-inner").getAttribute("required"), "Required property was propagated");
assert.strictEqual(browser.$("#input-required").shadow$(".ui5-input-inner").getAttribute("aria-required"), "true", "Aria-required attribute is set correctly");
assert.strictEqual(browser.$("#input-number").shadow$(".ui5-input-inner").getAttribute("aria-required"), "false", "Aria-required attribute is set correctly");
});

it("Type attribute is propagated properly", () => {
Expand Down
3 changes: 2 additions & 1 deletion packages/main/test/specs/TextArea.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@ describe("Attributes propagation", () => {
});

it("Required attribute is propagated properly", () => {
assert.ok(browser.$("#required-textarea").shadow$("textarea").getAttribute("required"), "Required property was propagated");
assert.strictEqual(browser.$("#required-textarea").shadow$("textarea").getAttribute("aria-required"), "true", "Aria-required attribute is set correctly");
assert.strictEqual(browser.$("#basic-textarea").shadow$("textarea").getAttribute("aria-required"), "false", "Aria-required attribute is set correctly");
});

it("Value attribute is propagated properly", () => {
Expand Down

0 comments on commit 7456ab5

Please sign in to comment.