docs: menuPortalZIndex docs improvements
#2484
Conversation
🦋 Changeset detectedLatest commit: e82b573 The changes in this PR will be included in the next version bump. This PR includes changesets to release 93 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
| // this IIFE is only to make the `menuPortalTarget` knob show up after `menuPortalZIndex` | ||
| {...(() => { | ||
| const menuPortalTarget = select( | ||
| 'menuPortalTarget', | ||
| ['undefined', 'document.body'], | ||
| 'undefined' | ||
| ); | ||
| return { | ||
| menuPortalTarget: | ||
| getMenuPortalTargetValue(menuPortalTarget), | ||
| }; | ||
| })()} | ||
| /> | ||
| {/* this IIFE is only to make the `menuPortalZIndex-show-neighbouring-stacking-context` knob show up last on the list */} | ||
| {(() => { | ||
| const isActive = boolean( | ||
| 'menuPortalZIndex-show-neighbouring-stacking-context', | ||
| false | ||
| ); | ||
| return ( | ||
| isActive && ( | ||
| <div | ||
| style={{ | ||
| width: '300px', | ||
| height: '50px', | ||
| position: 'relative', | ||
| zIndex: 2, | ||
| }} | ||
| > | ||
| <Card theme="dark"> | ||
| Stacking context with <code>z-index: 2</code> | ||
| </Card> | ||
| </div> | ||
| ) | ||
| ); | ||
| })()} |
There was a problem hiding this comment.
I wanted to keep those knobs close to each other, I don’t know any other way of providing certain order of knobs. This is the way I thought of.
Also, would you like me to copy this implementation in all other stories where we test components that have this prop?
There was a problem hiding this comment.
I believe It will also be helpful in other stories.
| @@ -76,7 +76,7 @@ export default Example; | |||
| | `hasWarning` | `boolean` | | | Indicates the input field has a warning | | |||
| | `maxMenuHeight` | `AsyncCreatableProps['maxMenuHeight']` | | | Maximum height of the menu before scrolling
<br>
[Props from React select was used](https://react-select.com/props) | | |||
| | `menuPortalTarget` | `AsyncCreatableProps['menuPortalTarget']` | | | Dom element to portal the select menu to
<br>
[Props from React select was used](https://react-select.com/props) | | |||
| | `menuPortalZIndex` | `number` | | | z-index value for the menu portal | | |||
| | `menuPortalZIndex` | `number` | | | z-index value for the menu portal
<br>
Use in conjunction with `menuPortalTarget` | | |||
There was a problem hiding this comment.
I am not adding a changeset as this PR is mainly updating READMEs and a story(ies).
kark
force-pushed
the
kk-menu-portal-z-index-docs-improvements
branch
from
12521d9
to
620e115
Compare
|
Nice work @kark. What do you think about warning the user when |
Thanks, yes, I think it makes sense to me. I'm a little concerned that this might blow up in several places in the MC-FE 😅 But unless anyone is strongly against, let's go for it and try to fix all the occurrences in the MC. |
It will only blow up if |
Thanks, good point👍 I added the warnings here but now I think we shouldn’t do that. For some reason we pass |
kark
force-pushed
the
kk-menu-portal-z-index-docs-improvements
branch
from
68afd76
to
51072ae
Compare
During the discussion yesterday we decided to keep the warning, but exclude I think this is ready for review now. |
Summary
closes #2477
Perhaps there is some misunderstanding on my part and those improvements are not needed at all.