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

feature(ui5-messagestrip): allow any content for the icon #1216

Merged
merged 2 commits into from
Feb 13, 2020

Conversation

vladitasev
Copy link
Contributor

  • The icon property is now a slot, therefore the user must provide the icon directly
  • This also allows for other content (such as an animated gif) to be used instead of an icon: see the samples
  • The noIcon property is unchanged - it still takes precedence over the icon, no matter that it is now a slot
  • The positioning of the icon is now done via a placeholder div, rather than by applying a class directly on the icon

BREAKING CHANGE: The icon property is now replaced with the icon slot. If you used a custom icon, for example:
<ui5-messagestrip icon="palette">
you should now pass an icon instance directly:
<ui5-messagestrip><ui5-icon slot="icon" name="palette"></ui5-icon>

packages/main/src/MessageStrip.js Outdated Show resolved Hide resolved
* @defaultvalue ""
* @public
*/
"icon": {
Copy link
Contributor

Choose a reason for hiding this comment

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

If the user can pass everything, we can think of a more generic name. Something like graphicalContent or visualContent

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think icon is fine, because it is consistent with all other components. It implies that you are expected to pass an icon normally, but as it is HTMLElement, you can pass anything technically.

@vladitasev vladitasev merged commit 5d4e594 into master Feb 13, 2020
@vladitasev vladitasev deleted the message-strip-icon-slot branch February 13, 2020 14:45
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.

2 participants