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

fix(ui5-icon): ui5-icon i18n works for all packages #2816

Merged
merged 3 commits into from
Feb 15, 2021
Merged

Conversation

vladitasev
Copy link
Contributor

@vladitasev vladitasev commented Feb 12, 2021

This change removes the dependency of ui5-icon to the @ui5/webcomponents-icons package.

Changes:

  • Each icon is now registered with the package name (new packageName property in icon data)
  • Icon.js no longer fetches automatically the i18n assets for @ui5/webcomponents-icons, but rather fetches the ones for the currently loaded icon's package. This would make it possible to have other icon packages with i18n data rather than the first icons package only.
  • For Icon.js the accessibleNameText is no longer a getter, but rather a metadata entity. This allows the Icon to be automatically invalidated once the i18n assets have been fetched and the acc text calculated.

Additional information: The first icon on the page may be invalidated up to 2 times. Firstly, if there is no data for the icon (the app did not load the icon individually), there will be a fetch for the icons bundle, and then the icon will be invalidated and actually painted with the SVG. Secondly, now that we have the icon's data, if the icon has an ACC text, and the app did not explicitly fetch the i18n bundle for the icon's package (which will almost always be the case), there will be a second fetch. After that - no requests will be done and all subsequent icons will be loaded normally.

closes: #2739

@vladitasev vladitasev merged commit 91e16a1 into master Feb 15, 2021
@vladitasev vladitasev deleted the icon-i18n-fix branch February 15, 2021 09:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ui5-icon should work with message bundles from the actual icons package
2 participants