Refactor popover positioning service #1
Merged
Conversation
This file contains 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
…steps: 1) Find original cross-axis position. 2) Apply shift to cross-axis position, if necessary. 3) Find primary axis position.
…imaryAxisPosition helper functions.
cjcenizal
commented
Jun 18, 2018
|
|
||
| const { | ||
| bestPosition, | ||
| } = iterationPositions.reduce(({ bestFit, bestPosition }, iterationPosition) => { |
There was a problem hiding this comment.
I like the encapsulation of using reduce because you don't need to mutate external variables, but it's a little inefficient because it always loops 4 times, even if the best fit has already been found. I don't mind if you want to switch this back to a for loop.
There was a problem hiding this comment.
I really like seeing it this way
|
|
||
| // the popover's original position on the cross-axis is determined by: | ||
| const crossAxisPositionOriginal = | ||
| anchorBoundingBox[crossAxisFirstSide] // where the anchor is located |
chandlerprall
pushed a commit
that referenced
this pull request
Apr 3, 2019
Altered styles and added some props
chandlerprall
pushed a commit
that referenced
this pull request
Aug 23, 2019
Update defazio fork to 13.0
chandlerprall
pushed a commit
that referenced
this pull request
Oct 25, 2019
chandlerprall
pushed a commit
that referenced
this pull request
Oct 28, 2019
Thread generics throughout EuiBasicTable
chandlerprall
pushed a commit
that referenced
this pull request
Feb 28, 2020
* removed duplicate icons * Fixing changelog and updating tests (#1) Co-authored-by: Caroline Horn <[email protected]>
chandlerprall
pushed a commit
that referenced
this pull request
Jan 26, 2021
fix prettier / ts issues
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
I broke this PR into the following steps, with corresponding commits:
getPopoverScreenCoordinatesinto discrete steps:a) Find original cross-axis position.
b) Apply shift to cross-axis position, if necessary.
c) Find primary axis position.
getCrossAxisPositionandgetPrimaryAxisPositionhelper functions, which also encapsulate the arrow-positioning logic for each axis. The primary axis arrow positioning logic was both adding and subtractingpositioning[primaryAxisPositionName], so I removed those operations.iterationPositionsarray ingetPopoverScreenCoordinates, which we traverse to get the best-fitting position, instead of defining the next iteration position in each loop. I found it easier to understand the strategy this way.