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

fix(Tooltip): Fixing incorrect tooltip shadow #1825

Merged
merged 6 commits into from
Aug 21, 2019

Conversation

bcalvery
Copy link
Contributor

@bcalvery bcalvery commented Aug 19, 2019

Changed TooltipContentVariables to use shadow value defined in siteVariables rather than separate values defined within tooltip.

Before:
image
After:
image

@codecov
Copy link

codecov bot commented Aug 19, 2019

Codecov Report

Merging #1825 into master will increase coverage by <.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1825      +/-   ##
==========================================
+ Coverage   69.56%   69.57%   +<.01%     
==========================================
  Files         875      874       -1     
  Lines        7596     7595       -1     
  Branches     2219     2219              
==========================================
  Hits         5284     5284              
+ Misses       2304     2303       -1     
  Partials        8        8
Impacted Files Coverage Δ
...rast/components/Tooltip/tooltipContentVariables.ts 0% <ø> (ø) ⬆️
...s/teams/components/Tooltip/tooltipContentStyles.ts 11.11% <ø> (ø) ⬆️
...eams/components/Tooltip/tooltipContentVariables.ts 0% <ø> (ø) ⬆️

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 881d670...35a93e4. Read the comment docs.

@codecov
Copy link

codecov bot commented Aug 19, 2019

Codecov Report

Merging #1825 into master will increase coverage by 0.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1825      +/-   ##
==========================================
+ Coverage   69.49%   69.51%   +0.01%     
==========================================
  Files         875      874       -1     
  Lines        7602     7603       +1     
  Branches     2214     2214              
==========================================
+ Hits         5283     5285       +2     
+ Misses       2311     2310       -1     
  Partials        8        8
Impacted Files Coverage Δ
...rast/components/Tooltip/tooltipContentVariables.ts 0% <ø> (ø) ⬆️
...s/teams/components/Tooltip/tooltipContentStyles.ts 11.11% <ø> (ø) ⬆️
...eams/components/Tooltip/tooltipContentVariables.ts 0% <ø> (ø) ⬆️
...kages/react/src/themes/teams-dark/siteVariables.ts 100% <100%> (ø) ⬆️

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 1556539...288c091. Read the comment docs.

CHANGELOG.md Outdated
@@ -31,6 +31,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
- Fix `hand` icon in Teams theme @lucivpav ([#1782](https://github.com/stardust-ui/react/pull/1782))
- ESC key should close the last opened `Popup` or `Dialog` if body has focus @sophieH29 ([#1807](https://github.com/stardust-ui/react/pull/1807))
- Correctly define current document object of the `FocusZone` and `FocusTrapZone` @sophieH29 ([#1820](https://github.com/stardust-ui/react/pull/1820))
- Fix `Tooltip` shadow incorrect [redlines] @bcalvery ([#1825](https://github.com/stardust-ui/react/pull/1825))
Copy link
Collaborator

Choose a reason for hiding this comment

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

@miroslavstastny, does a variable rename need to be in the BREAKING CHANGES section?

Copy link
Member

Choose a reason for hiding this comment

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

@codepretty yep, as it's a part of public API

Copy link
Collaborator

@codepretty codepretty left a comment

Choose a reason for hiding this comment

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

Just make sure that it is in the right spot in the changelog. Rest of it looks good to me!

@bcalvery bcalvery merged commit 7a37ac0 into master Aug 21, 2019
@delete-merged-branch delete-merged-branch bot deleted the fix/tooltip-shadow-darktheme branch August 21, 2019 21:50
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants