Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add KSelect dropdown to the Overlay layer #877

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

AlexVelezLl
Copy link
Member

@AlexVelezLl AlexVelezLl commented Dec 20, 2024

Description

Finally, a robust solution is proposed to the many known issues with the KSelect dropdown by teleporting KSelect dropdown to the Overlay layer using Popper.

Issue addressed

Addresses #324 and #690.

Before/after screenshots

Before After
image image
image image
image image
If a modal had a long form, it bugged because of the overflowY: unset set because the Modal check for KSelects Now, we can have scrollables modals even if it has a KSelect inside
image Screenshot from 2024-12-26 16-01-41

Screencast showing the behaviour of KSelect in a long modal:

Compartir.pantalla.-.2024-12-26.16_10_21.mp4

Changelog

Steps to test

  1. Step 1
  2. Step 2
  3. ...

(optional) Implementation notes

At a high level, how did you implement this?

I have decided to close the dropdown on scroll, just like the KDropdownMenu, to avoid situations like this:

image

Otherwise we would need to be querying scrollable ancestors elements, and taking into account the height of any absolute/fixed elements like the topbar to automaticlly close the dropdown.

Does this introduce any tech-debt items?

Testing checklist

  • Contributor has fully tested the PR manually
  • If there are any front-end changes, before/after screenshots are included
  • Critical and brittle code paths are covered by unit tests
  • The change is described in the changelog section above

Reviewer guidance

  • Is the code clean and well-commented?
  • Are there tests for this change?
  • Are all UI components LTR and RTL compliant (if applicable)?
  • Add other things to check for here

Comments

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant