This repository was archived by the owner on Mar 4, 2020. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 53
feat(Popup): add 'offset' prop #606
Merged
Merged
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
31111c4
introduce offset prop
kuzhelov-ms ccf678c
correct description of supported values
kuzhelov-ms 557aeae
Merge branch 'master' into feat/add-offset-prop-to-popup
kuzhelov 47ad701
update changelog
kuzhelov-ms 166117e
Merge branch 'master' into feat/add-offset-prop-to-popup
kuzhelov File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
44 changes: 44 additions & 0 deletions
44
docs/src/examples/components/Popup/Variations/PopupExampleOffset.shorthand.tsx
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,44 @@ | ||
import React from 'react' | ||
import { Button, Grid, Popup, Alignment, Position } from '@stardust-ui/react' | ||
|
||
const renderButton = rotateArrowUp => ( | ||
<Button | ||
icon={{ | ||
name: 'arrow circle up', | ||
styles: { transform: `rotate(${rotateArrowUp})` }, | ||
}} | ||
styles={{ height: '80px', minWidth: '80px', padding: 0 }} | ||
/> | ||
) | ||
|
||
const triggers = [ | ||
{ position: 'above', align: 'start', offset: '-100%p', rotateArrowUp: '-45deg' }, | ||
{ position: 'above', align: 'end', offset: '100%p', rotateArrowUp: '45deg' }, | ||
{ position: 'below', align: 'start', offset: '-100%p', rotateArrowUp: '-135deg' }, | ||
{ position: 'below', align: 'end', offset: '100%p', rotateArrowUp: '135deg' }, | ||
] | ||
|
||
const PopupExamplePosition = () => ( | ||
<Grid columns="repeat(2, 80px)" variables={{ padding: '30px', gridGap: '30px' }}> | ||
{triggers.map(({ position, align, offset, rotateArrowUp }) => ( | ||
<Popup | ||
align={align as Alignment} | ||
position={position as Position} | ||
offset={offset} | ||
trigger={renderButton(rotateArrowUp)} | ||
content={{ | ||
content: ( | ||
<p> | ||
The popup is rendered at {position}-{align} | ||
<br /> | ||
corner of the trigger. | ||
</p> | ||
), | ||
}} | ||
key={`${position}-${align}`} | ||
/> | ||
))} | ||
</Grid> | ||
) | ||
|
||
export default PopupExamplePosition |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Too bad applying modifiers is the only way we can adjust the offset... There is no way we can use this prop inside the styles in order to achieve the position, right? It really feels awkward to have to tackle rtl in the component's logic..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, there is no any - so, there are several reasons for that.
First is simple - there are no additional props of Popper that would allow client to customize positioning (actually, good thing). And here we are getting to the next alternative option that might come to mind (the one you've mentioned) - use styles for influencing these offsets.
I've tried this one - and have done that only to clearly see that even in LTR mode taken in isolation we have lots of issues. As an example: if it is about 'start' value for
align
property, which corresponds to popup aligned with the left side of trigger,we might use
left
to specify an offset (asabsolute
positioning strategy is used by Popper). However, when we will achieve necessary result and switch to the ride sides alignment, we will see that Popper is usingtransform
property to ensure alignment for the right sides, and here we cannot use offsets ofleft-right
CSS properties anymore.For the sake of not preventing havoc to wreak our code, decided to just reuse solution Popper has already introduced for solving exactly this problem (probably, the most reasonable thing) - and this, in fact, what we've done with the modifiers. Note, we are not abusing it - we are using the tool from Popper that is, actually, dedicated to solve this exact problem :)