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: (Core) add semantic icons to Message box and remove the close button #4128

Merged
merged 4 commits into from
Dec 22, 2020

Conversation

InnaAtanasova
Copy link
Contributor

Please provide a link to the associated issue.

Closes #3887

Please provide a brief summary of this pull request.

  • introduced semantic icons into Message box component
  • removed the close button as per Fiori specs
  • the Switch component is broken but it comes from v.0.14.0 of Fundamental-styles and is fixed in this PR

BREAKING CHANGE:
MessageBoxCloseIconComponent component is removed

Please check whether the PR fulfills the following requirements

Documentation checklist:

@InnaAtanasova InnaAtanasova added enhancement New feature or request denoland labels Dec 16, 2020
@InnaAtanasova InnaAtanasova added this to the Sprint 52 - Hilo milestone Dec 16, 2020
@InnaAtanasova InnaAtanasova requested a review from a team December 16, 2020 22:14
@InnaAtanasova InnaAtanasova self-assigned this Dec 16, 2020
@netlify
Copy link

netlify bot commented Dec 16, 2020

✔️ Deploy preview for fundamental-ngx ready!

🔨 Explore the source changes: fc93cac

🔍 Inspect the deploy logs: https://app.netlify.com/sites/fundamental-ngx/deploys/5fdcf3fe6bf65d00084f412f

😎 Browse the preview: https://deploy-preview-4128--fundamental-ngx.netlify.app

@InnaAtanasova
Copy link
Contributor Author

InnaAtanasova commented Dec 16, 2020

NOTE: tests are failing for FlexibleColumnLayout but they are fixed in 2 other PRs (just need someone to review them :) )
#4108 and #4116

As mentioned in the template, Switch component is broken but it's due to v.0.14.0 which is fixed in #4116

Copy link
Contributor

@salarenko salarenko left a comment

Choose a reason for hiding this comment

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

  1. RIP close button. Do we have to remove this feature? It looks like it also should't be supported in the Dialog component, but we keep it there. It's weird that the design excluded such common element.
  2. Should we force user to put the icon element in the header by himself? Maybe we could put it always in the default message-box header template and show the user how to compose it by himself in the " Complex template" example?

@JKMarkowski JKMarkowski changed the base branch from master to main December 18, 2020 17:25
@JKMarkowski JKMarkowski merged commit 3a5aa87 into main Dec 22, 2020
@JKMarkowski JKMarkowski deleted the fix/message-box-updates branch December 22, 2020 12:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
denoland enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Message box semantic icons
4 participants