Skip to content
This repository has been archived by the owner on Mar 4, 2020. It is now read-only.

feat(popup): unstable_pinned prop #1471

Merged
merged 4 commits into from
Jun 11, 2019
Merged

Conversation

bmdalex
Copy link
Collaborator

@bmdalex bmdalex commented Jun 7, 2019

feat(popup): unstable_pinned prop

Description

There are certain use cases where our users need to position Popup content or Dropdown items using a specific alignment and position (via align and position props).

By default, Popup content or Dropdown items reposition themselves automatically on both axis if the content / items do not fit in the viewport. However, the algorithm has a few issues, outlined in #1391 so this PR addresses that with a short term fix presented here:

The short term fix is to add the unstable_pinned prop that disables automatic repositioning of the component; it will always be placed according to the values of align and position props, regardless of the size of the component, the reference element or the viewport.

e.g.

<Popup align="start" position="above" pinned ... />

will create a Popup that will always be placed on top of the trigger aligned to the start (auto positioning will be turned off). Same applies for Dropdown.

Screenshot outlining the difference in Popup:

Screen Recording 2019-06-07 at 14 23 45

@bmdalex
Copy link
Collaborator Author

bmdalex commented Jun 7, 2019

fyi @kuzhelov @layershifter @stuartlong

@codecov
Copy link

codecov bot commented Jun 7, 2019

Codecov Report

Merging #1471 into master will decrease coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1471      +/-   ##
==========================================
- Coverage   73.47%   73.46%   -0.01%     
==========================================
  Files         807      806       -1     
  Lines        6088     6087       -1     
  Branches     1752     1800      +48     
==========================================
- Hits         4473     4472       -1     
+ Misses       1610     1609       -1     
- Partials        5        6       +1
Impacted Files Coverage Δ
packages/react/src/lib/positioner/Popper.tsx 89.13% <100%> (-0.46%) ⬇️
...ackages/react/src/components/Dropdown/Dropdown.tsx 88.54% <100%> (ø) ⬆️
packages/react/src/components/Popup/Popup.tsx 67.74% <100%> (ø) ⬆️
packages/react/src/components/Tree/TreeTitle.tsx 50% <0%> (-36.37%) ⬇️
packages/react/src/components/Tree/Tree.tsx 71.42% <0%> (-21.91%) ⬇️
...lib/accessibility/Behaviors/Popup/popupBehavior.ts 89.65% <0%> (-6.18%) ⬇️
...haviors/Toolbar/menuItemAsToolbarButtonBehavior.ts 100% <0%> (ø) ⬆️
...ssibility/Behaviors/List/selectableListBehavior.ts 100% <0%> (ø) ⬆️
...c/lib/accessibility/Behaviors/Icon/iconBehavior.ts 100% <0%> (ø) ⬆️
...b/accessibility/Behaviors/List/listItemBehavior.ts 100% <0%> (ø) ⬆️
... and 23 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e28114e...c8808cc. Read the comment docs.

@bmdalex
Copy link
Collaborator Author

bmdalex commented Jun 7, 2019

closing for now as @layershifter will introduce a better solution

@bmdalex bmdalex closed this Jun 7, 2019
@bmdalex bmdalex reopened this Jun 7, 2019
@bmdalex bmdalex force-pushed the feat/popup-pinned-positioning branch from 4cfe0bc to 0f9b1bf Compare June 7, 2019 16:03
@bmdalex
Copy link
Collaborator Author

bmdalex commented Jun 7, 2019

in the end we need this PR, added few more changes

@bmdalex bmdalex force-pushed the feat/popup-pinned-positioning branch 2 times, most recently from 8f592a6 to d39a5e3 Compare June 10, 2019 14:18
@bmdalex bmdalex force-pushed the feat/popup-pinned-positioning branch from d39a5e3 to c8808cc Compare June 11, 2019 14:46
@bmdalex bmdalex merged commit eeb3825 into master Jun 11, 2019
@delete-merged-branch delete-merged-branch bot deleted the feat/popup-pinned-positioning branch June 11, 2019 15:32
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants