Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 15 commits into from
Aug 30, 2019
Merged

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

merged 15 commits into from
Aug 30, 2019

Conversation

tsanislavgatev
Copy link
Contributor

No description provided.

@@ -3,7 +3,7 @@
dir="{{rtl}}"
>
<div class="{{classes.header}}" id="{{_id}}-header">
<ui5-icon @click="{{_onHeaderBackArrowClick}}" class="{{classes.headerBackArrow}}" src="sap-icon://slim-arrow-left" tabindex="-1"></ui5-icon>
<ui5-icon @click="{{_onHeaderBackArrowClick}}" class="{{classes.headerBackArrow}}" src="sap-icon://slim-arrow-left" tabindex="-1" accessible-name="Previous" icon-tooltip="true"></ui5-icon>
Copy link
Member

@ilhan007 ilhan007 Aug 30, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have to ensure that the "Previous" and "Next" texts would be translated. In ui5-webcomponents we have similar translation process as in openui5.

(1) You have to add those texts in the packages/main/src/18n/messagebundle.properties file in first place (every week on Wednesday a translation team delivers translations for new texts ).
TABCONTAINER_NEXT_ICON_ACC_NAME: "Next"

As we do not watch for changes in .properties files, because they occur rarely, you probably have to stop and run yarn start again. If everything is fine, you can find the newly added text
in /packages/main/dist/generated/i18n/i18n-defaults.js

(2) You have to use the i18nBundle.js and i18n-defaults.js in TabContainer.js
import { fetchI18nBundle, getI18nBundle } from "@ui5/webcomponents-base/dist/i18nBundle.js";
import { TABCONTAINER_NEXT_ICON_ACC_NAME } from "./generated/i18n/i18n-defaults.js";

(3) Create a getter for the text:

 // TabContainer.js
get nextIconACCName() {
     return this.i18nBundle.getText(TABCONTAINER_NEXT_ICON_ACC_NAME);
 }

(4) And use the getter in the .hbs file:

<ui5-icon  accessible-name="{nextIconACCName}" />

You can see how to use the i18nBundle in the Badge.js, so use it for reference.
In case you need more information how to do it, feel free to contact us.

Copy link
Contributor

@elenastoyanovaa elenastoyanovaa Aug 30, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is also applicable to the title="Overflow Menu". You should translate it as Ilhan mentioned

@ilhan007
Copy link
Member

We have guidelines for commit messages (https://conventionalcommits.org/), you can briefly look at our previous releases to get examples: https://github.com/SAP/ui5-webcomponents/releases

In this case something like this is appropriate:
refactor(ui5-tabcontainer): improve/update ACC of next/previous icons
feat(ui5-tabcontainer): implement ACC of next/previous icons

@@ -3,7 +3,7 @@
dir="{{rtl}}"
>
<div class="{{classes.header}}" id="{{_id}}-header">
<ui5-icon @click="{{_onHeaderBackArrowClick}}" class="{{classes.headerBackArrow}}" src="sap-icon://slim-arrow-left" tabindex="-1"></ui5-icon>
<ui5-icon @click="{{_onHeaderBackArrowClick}}" class="{{classes.headerBackArrow}}" src="sap-icon://slim-arrow-left" tabindex="-1" accessible-name="Previous" icon-tooltip="true"></ui5-icon>
Copy link
Contributor

@elenastoyanovaa elenastoyanovaa Aug 30, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is also applicable to the title="Overflow Menu". You should translate it as Ilhan mentioned

@@ -45,7 +45,7 @@
</ul>
</div>

<ui5-icon @click="{{_onHeaderForwardArrowClick}}" class="{{classes.headerForwardArrow}}" src="sap-icon://slim-arrow-right" tabindex="-1"></ui5-icon>
<ui5-icon @click="{{_onHeaderForwardArrowClick}}" class="{{classes.headerForwardArrow}}" src="sap-icon://slim-arrow-right" tabindex="-1" accessible-name="Next" icon-tooltip="true"></ui5-icon>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually we changed the icon-tooltip to show-tooltip. You see the documentation of the Icon component

@@ -3,7 +3,7 @@
dir="{{rtl}}"
>
<div class="{{classes.header}}" id="{{_id}}-header">
<ui5-icon @click="{{_onHeaderBackArrowClick}}" class="{{classes.headerBackArrow}}" src="sap-icon://slim-arrow-left" tabindex="-1"></ui5-icon>
<ui5-icon @click="{{_onHeaderBackArrowClick}}" class="{{classes.headerBackArrow}}" src="sap-icon://slim-arrow-left" tabindex="-1" accessible-name="Previous" icon-tooltip="true"></ui5-icon>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

icon-tooltip should be replaced with show-tooltip

@ilhan007 ilhan007 changed the title ACC TabContainer component update feat(ui5-tabcontainer): update ACC of header and content Aug 30, 2019
@@ -3,7 +3,7 @@
dir="{{rtl}}"
>
<div class="{{classes.header}}" id="{{_id}}-header">
<ui5-icon @click="{{_onHeaderBackArrowClick}}" class="{{classes.headerBackArrow}}" src="sap-icon://slim-arrow-left" tabindex="-1" accessible-name="Previous" icon-tooltip="true"></ui5-icon>
<ui5-icon @click="{{_onHeaderBackArrowClick}}" class="{{classes.headerBackArrow}}" src="sap-icon://slim-arrow-left" tabindex="-1" accessible-name="{{previousIconACCName}}" show-tooltip="true"></ui5-icon>

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

show-tooltip should be enough as it is a Boolean property, replace show-tooltip="true" with show-tooltip here and below

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

TABCONTAINER_NEXT_ICON_ACC_NAME=Next

#XACT: ACC previous icon name in tab container
TABCONTAINER_PREVIOUS_ACC_NAME=Previous
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just this small thing, let's be consistent,
we have
TABCONTAINER_NEXT_ICON_ACC_NAME, but TABCONTAINER_PREVIOUS_ACC_NAME (without icon in the name).

Rename it to TABCONTAINER_PREVIOUS_ICON_ACC_NAME and don`t forget to update the place you import them.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@ilhan007 ilhan007 merged commit 8550365 into SAP:master Aug 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants