feat(ui5-icon): accessibility implementation #709
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
vladitasev
reviewed
Aug 2, 2019
packages/main/src/Icon.js
Outdated
| * @defaultvalue false | ||
| * @public | ||
| */ | ||
| showIconTooltip: { |
There was a problem hiding this comment.
IMO just showTooltip (Icon is in the name of the element already)
There was a problem hiding this comment.
I was thinking a lot about this name! Thanks, I have changed it :)
vladitasev
previously approved these changes
Aug 2, 2019
vladitasev
previously requested changes
Aug 5, 2019
|
|
||
| const name = "sap-icon://message-information"; | ||
| const d = "M448 32q13 0 22.5 9t9.5 22v384q0 14-9.5 23.5T448 480H64q-14 0-23-9.5T32 447V63q0-13 9-22t23-9h384zm0 31H64v384h384V63zM320 416H192v-33h33V255h-32v-31h95v159h32v33zm-64-225q-14 0-23-9t-9-22q0-14 9-23.5t23-9.5q13 0 22.5 9.5T288 160q0 13-9.5 22t-22.5 9z"; | ||
| const acc = ICON_MESSAGE_INFORMATION.defaultText; |
There was a problem hiding this comment.
Should be the whole object, not the default text, so that it can be translated
packages/main/src/Icon.js
Outdated
| get accessibleNameText() { | ||
| const icon = getIconData(this._normalizeIconURI(this.src)); | ||
|
|
||
| return this.accessibleName || icon.accText; |
There was a problem hiding this comment.
getResourceBundle().getText() should be here
There was a problem hiding this comment.
We have to fix it first to not break. It should return the english text if no text is found in the translation
vladitasev
approved these changes
Aug 7, 2019
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.