Skip to content
This repository was archived by the owner on Mar 4, 2020. It is now read-only.

fix(Alert): themeing updates for the alert component#1226

Merged
codepretty merged 29 commits intomasterfrom
alert-banner-updates
Apr 26, 2019
Merged

fix(Alert): themeing updates for the alert component#1226
codepretty merged 29 commits intomasterfrom
alert-banner-updates

Conversation

@codepretty
Copy link
Collaborator

@codepretty codepretty commented Apr 16, 2019 •
edited
Loading

  1. Updated the themeing colors for Alert component.
  2. Added new variations for OOF and Urgent banner types and added examples in the docs for these new types.

Themeing update for Alert component
image

@codecov
Copy link

codecov bot commented Apr 16, 2019 •
edited
Loading

Codecov Report

Merging #1226 into master will decrease coverage by 0.13%.
The diff coverage is 7.69%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #1226      +/-   ##
=========================================
- Coverage   72.14%     72%   -0.14%     
=========================================
  Files         731     734       +3     
  Lines        5596    5608      +12     
  Branches     1614    1618       +4     
=========================================
+ Hits         4037    4038       +1     
- Misses       1554    1565      +11     
  Partials        5       5
Impacted Files Coverage Δ
...rc/themes/teams/components/Alert/alertVariables.ts 0% <ø> (ø) ⬆️
...s-high-contrast/components/Alert/alertVariables.ts 0% <0%> (ø)
...t/src/themes/teams/components/Alert/alertStyles.ts 8% <0%> (-2%) ⬇️
...emes/teams-dark/components/Alert/alertVariables.ts 0% <0%> (ø)
...eams-high-contrast/components/Alert/alertStyles.ts 33.33% <33.33%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 34fc84c...2fc3bf5. Read the comment docs.

@codepretty codepretty requested a review from notandrew April 16, 2019 00:46
@codepretty codepretty added 🚀 ready for review question Further information is requested, concerns that require additional thought are raised redlines Update of the redlines for the mentioned component labels Apr 16, 2019
@codepretty codepretty requested a review from jaanus03 April 16, 2019 00:47
miroslavstastny
miroslavstastny reviewed Apr 17, 2019
View reviewed changes
@miroslavstastny
Copy link
Member

miroslavstastny commented Apr 17, 2019 •
edited
Loading

***Does anyone know why the popout page feature returns a 404 for the OOF variation?

Maximized view breaks if example filename contains multiple consecutive capital letters (OOF in this case) as a workaround please rename the example files to AlertExampleOof.

adding '[redlines]' indicator to signify to TMP side that style overr…
431818a 
…ides may need to be removed for this change
bmdalex
bmdalex reviewed Apr 17, 2019
View reviewed changes
bmdalex
bmdalex approved these changes Apr 17, 2019
View reviewed changes
Copy link
Collaborator

@bmdalex bmdalex left a comment

Choose a reason for hiding this comment

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

looks good, few comments 👍

@codepretty
Copy link
Collaborator Author

codepretty commented Apr 17, 2019

agreed with @mnajdova , oof and urgent should be variables for now
@codepretty what happened with critical variation?

@Bugaa92 critical merged into urgent

@codepretty codepretty removed the question Further information is requested, concerns that require additional thought are raised label Apr 17, 2019
@kuzhelov kuzhelov added needs author feedback Author's opinion is asked and removed 🚀 ready for review labels Apr 18, 2019
@codepretty codepretty removed the needs author feedback Author's opinion is asked label Apr 20, 2019
mnajdova
mnajdova reviewed Apr 24, 2019
View reviewed changes
packages/react/src/themes/teams/components/Alert/alertVariables.ts Outdated
focusInnerBorderColor: siteVars.colors.white,
focusOuterBorderColor: siteVars.colors.black,

dangerColor: siteVars.red,
Copy link
Contributor

@mnajdova mnajdova Apr 24, 2019

Choose a reason for hiding this comment

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

Can we add comments here about what should this color be with the new palette?

@codepretty codepretty merged commit 06f785a into master Apr 26, 2019
@delete-merged-branch delete-merged-branch bot deleted the alert-banner-updates branch April 26, 2019 10:48
@alinais alinais mentioned this pull request Jul 24, 2019
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@dvdzkwsk dvdzkwsk Awaiting requested review from dvdzkwsk

@kuzhelov kuzhelov Awaiting requested review from kuzhelov

@layershifter layershifter Awaiting requested review from layershifter layershifter is a code owner

@levithomason levithomason Awaiting requested review from levithomason levithomason is a code owner

@notandrew notandrew Awaiting requested review from notandrew

@jaanus03 jaanus03 Awaiting requested review from jaanus03

3 more reviewers

@bmdalex bmdalex bmdalex approved these changes

@miroslavstastny miroslavstastny miroslavstastny left review comments

@mnajdova mnajdova mnajdova approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

redlines Update of the redlines for the mentioned component

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@codepretty @miroslavstastny @mnajdova @bmdalex @kuzhelov