Skip to content
This repository has been archived by the owner on Jul 10, 2023. It is now read-only.

Fix: dropdown overflow #335

Closed
wants to merge 15 commits into from
Closed

Fix: dropdown overflow #335

wants to merge 15 commits into from

Conversation

pKorsholm
Copy link
Contributor

@pKorsholm pKorsholm commented Feb 8, 2022

What

  • fix overflow issue with dropdown

How

  • switch dropdown component from react-multi-select-component to react-select (which has more users, stars and downloads in general)
  • react-select supports native portalling of the dropdown which enables us to show it as an overlay instead, which react-multi-select-component doesn't have
  • add ModalContext which exposes a ref to the modal portal

@netlify
Copy link

netlify bot commented Feb 8, 2022

✔️ Deploy Preview for elastic-keller-bcaedb ready!

🔨 Explore the source changes: 12cf4d1

🔍 Inspect the deploy log: https://app.netlify.com/sites/elastic-keller-bcaedb/deploys/620d3225580c100008c42743

😎 Browse the preview: https://deploy-preview-335--elastic-keller-bcaedb.netlify.app

@pKorsholm pKorsholm changed the base branch from feat/revamp to develop February 8, 2022 17:01
@pKorsholm pKorsholm changed the base branch from develop to feat/revamp February 8, 2022 17:02
@zakariaelas zakariaelas added the in review: zak currently in review by zak label Feb 10, 2022
@olivermrbl olivermrbl added this to In review in medusajs Feb 13, 2022
@olivermrbl olivermrbl added the type: feature or enhancement New feature or enhancement label Feb 14, 2022
@zakariaelas zakariaelas removed the in review: zak currently in review by zak label Feb 14, 2022
Copy link
Contributor

@kasperkristensen kasperkristensen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very cool! Howver, I noticed two issues.

You are able to take the dropdown for a walk when scrolling 😅

Screen.Recording.2022-02-15.at.10.02.27.mov

And if you open the dropdown using the chevron then search field is not focused and the dropdown will have a different position then when it the search fields is focussed:

Screen.Recording.2022-02-15.at.10.12.18.mov

@pKorsholm
Copy link
Contributor Author

pKorsholm commented Feb 15, 2022

The scrolling issue is a known issue: JedWatson/react-select#4088 (which was introduced in 3. something, we use 5. something else)

I have solved it by closing the dropdown when scrolling.

@kasperkristensen
Copy link
Contributor

kasperkristensen commented Feb 15, 2022

@pKorsholm Ahh cool, this brings it very close to being perfect. Closing it on scroll does result in some weird behaviour if the select is very close to the bottom of the viewable part of a scrollable div. This scrolls the window a bit and then closes the Select immediately

Screen.Recording.2022-02-15.at.16.03.44.mov

@olivermrbl
Copy link
Contributor

Also getting strange behaviour when trying to open other dropdowns, when one is already open. It seems, that in order for us to open a different dropdown and close the currently open one, we have to either scroll to close it, or close it by clicking on it.

See attached video for reference.

Screen.Recording.2022-02-16.at.11.23.04.mov

@olivermrbl olivermrbl moved this from In review to Requires changes in medusajs Feb 16, 2022
@pKorsholm
Copy link
Contributor Author

I think we are closing in 😄 changed up the open/close monitoring in favor of the built in controls and instead now monitor for focus and blur.

I fixed the issue with 'jump scrolling using a debounce such that scrolling in the first 50 ms after opening the dropdown will not close it. If you have a better suggestion please let me know, issue is that scroll events when opening the dropdown and regular scrolling are indistinguishable.

Screen.Recording.2022-02-16.at.16.03.08.mov

@pKorsholm pKorsholm moved this from Requires changes to In review in medusajs Feb 16, 2022
@olivermrbl
Copy link
Contributor

olivermrbl commented Feb 16, 2022

To somewhat re-use @kasperkristensen's epic phrase from above, I can't take the dropdown for a walk, if I resize the window 😅

image

@pKorsholm pKorsholm mentioned this pull request Feb 16, 2022
@pKorsholm pKorsholm moved this from In review to Done in medusajs Feb 16, 2022
@pKorsholm pKorsholm closed this Feb 16, 2022
@pKorsholm pKorsholm deleted the fix/dropdown-overflow branch February 16, 2022 18:17
@olivermrbl olivermrbl removed this from Done in medusajs Feb 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
type: feature or enhancement New feature or enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants