fix(Popup): fix styling with pointer when it is wrapped by FocusZones#1492
fix(Popup): fix styling with pointer when it is wrapped by FocusZones#1492layershifter merged 3 commits intomasterfrom
pointer when it is wrapped by FocusZones#1492Conversation
547bb2c to
0fbb5c2
Compare
0fbb5c2 to
7117c60
Compare
pointer when it is wrapped by FocusZones
Codecov Report
@@ Coverage Diff @@
## master #1492 +/- ##
==========================================
- Coverage 73.44% 73.43% -0.01%
==========================================
Files 822 822
Lines 6190 6192 +2
Branches 1795 1797 +2
==========================================
+ Hits 4546 4547 +1
- Misses 1639 1640 +1
Partials 5 5
Continue to review full report at Codecov.
|
mnajdova
left a comment
There was a problem hiding this comment.
Don't have better idea for now. We may actually add the wrapper prop, as it is in the menu, but we really need just a boolean here for now, so as a short term fix, I am okay with this.
CHANGELOG.md
Outdated
|
|
||
| ### Fixes | ||
| - Fix prop types of `Tooltip` component @kuzhelov ([#1499](https://github.com/stardust-ui/react/pull/1499)) | ||
| - Fix `Popup` styling with `pointer` when it is wrapped by FocusZones @layershifter ([#1492](https://github.com/stardust-ui/react/pull/1492)) |
There was a problem hiding this comment.
nit: lets use code formatting for FocusZones
| pointerRef?: React.Ref<Element> | ||
|
|
||
| /** | ||
| * Indicates that PopupContent is wrapped with FocusZone. Do not use it, it used only for internal implementation and |
There was a problem hiding this comment.
nit: lets use @deprecated pragma to make proper accent on that - as this pragma is commonly supported and visualised (at least) by IDEs
There was a problem hiding this comment.
Will do, but you accidentally tagged someone 😸
| display: 'block', | ||
|
|
||
| ...(p.unstable_wrapped && { | ||
| backgroundColor: 'inherit', |
There was a problem hiding this comment.
Wouldn't it be enough to add the styles all the time? without the condition of p.unstable_wrapped? I mean maybe we can avoid this prop altogether... Or will adding these styles cause different issue?
There was a problem hiding this comment.
If we will always add these styles we will inherit backgroundColor from body when there are no wrapper...
Fixes #1433.
Problem
When we use behaviors for
Popupthat have FocusZones, it results in newdivand changed DOM structure, which causes broken styles.Solution
Provide
wrappedprop toPopupContent, so we will know if it is wrapped by FocusZones and provide correct styles. This solution is short-term and aims only to fix bug - as any refactoring that will make it simpler and will allow to avoidwrappedprop will contain breaking changes, which we want to allow for now.I decided to prefer prop instead of passing down the
variablesbecause we don't have a proper merging of them.