feat(Popup): add pointing prop
#1198
Conversation
layershifter
requested review from
dvdzkwsk,
kuzhelov,
levithomason,
miroslavstastny and
mnajdova
as code owners
| position: 'absolute', | ||
| textAlign: 'left', | ||
| color: variables.contentColor, | ||
| background: variables.contentBackgroundColor, |
There was a problem hiding this comment.
Moved to popupContentStyles...
There was a problem hiding this comment.
these styles should be left here, as we popup expects any content to be rendered with this set of styles, not just PopupContent. Also, please, ensure that PopupContent just CSS-inherits this set of colors.
These two requirements are necessary for introducing correct semantics around these styles, as well as for expected effects of theme switching.
FYI #1121
layershifter
changed the title
pointing proppointing prop
packages/react/src/themes/teams/components/Popup/popupContentStyles.ts
Outdated
Show resolved
Hide resolved
packages/react/src/themes/teams/components/Popup/popupContentVariables.ts
Outdated
Show resolved
Hide resolved
packages/react/src/themes/teams/components/Popup/popupContentStyles.ts
Outdated
Show resolved
Hide resolved
…om/stardust-ui/react into feat/popup-pointing
Codecov Report
@@ Coverage Diff @@
## master #1198 +/- ##
=========================================
- Coverage 82.54% 82.5% -0.04%
=========================================
Files 751 751
Lines 8850 8865 +15
Branches 1246 1251 +5
=========================================
+ Hits 7305 7314 +9
- Misses 1530 1536 +6
Partials 15 15
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #1198 +/- ##
==========================================
- Coverage 82.57% 82.53% -0.05%
==========================================
Files 752 751 -1
Lines 8868 8880 +12
Branches 1186 1259 +73
==========================================
+ Hits 7323 7329 +6
- Misses 1530 1536 +6
Partials 15 15
Continue to review full report at Codecov.
|
| padding: string | ||
|
|
||
| contentColor: string |
There was a problem hiding this comment.
- there are
contentColorandcontentBackgroundColorleft inpopupVariablesas well and are not used inpopupStyles popupVariablesin teams-dark is settingcolorwhich is not used- if we move variables from
popuptopopupContentit is a breaking change
There was a problem hiding this comment.
contentColor and contentBackgroundColor are now variables of PopupContent. Is the content in the variables' names still meaningful?
| const PopupWithButton = props => ( | ||
| <Popup | ||
| align={props.align} | ||
| content="A popup with a pointer." |
There was a problem hiding this comment.
For the other popup example, we intentionally use multiline content to clearly show the difference between top/center/bottom - it would be great to have the same here as well.
There was a problem hiding this comment.
please, take a look on this comment about styles: https://github.com/stardust-ui/react/pull/1198/files#r273879017
…at/popup-pointing
…om/stardust-ui/react into feat/popup-pointing
This PR adds a new
pointingprop that allows to show a pointer to a trigger.pointingWe have
pointingprop inMenu, used this name for consistency.Changes in
PopupContentDOM structure
A
pointershould be a slot ofPopupContent, otherwise it fill fail. Bootstrap uses PopperJS and the same structure, BTW.contentslotAs I have to change DOM structure, I added a new slot
content, but it's really weird to havePopupContent.content😮innermay be?