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 KFocusTrap and use it in KModal #799

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

Conversation

lokesh-sagi125
Copy link
Contributor

Description

copy FocusTrap from kolibri into KFocusTrap in the KDS and use it in the KModal

Issue addressed

Add KFocusTrap and use it in KModal #746

Addresses #746

Before/after screenshots

before:-
https://github.com/user-attachments/assets/c38be827-683f-4d45-86b2-e845a2193744

after:-
https://github.com/user-attachments/assets/b5b27ea8-0d12-42fa-8fff-2de408a4eeb3

Changelog

  • Description: Summary of change(s)
  • Products impact: Choose from - none (for internal updates) / bugfix / new API / updated API / removed API. If it's 'none', use "-" for all items below to indicate they are not relevant.
  • Addresses: Link(s) to GH issue(s) addressed. Include KDS links as well as links to related issues in a consumer product repository too.
  • Components: Affected public KDS component. Do not include internal sub-components or documentation components.
  • Breaking: Will this change break something in a consumer? Choose from: yes / no
  • Impacts a11y: Does this change improve a11y or adds new features that can be used to improve it? Choose from: yes / no
  • Guidance: Why and how to introduce this update to a consumer? Required for breaking changes, appreciated for changes with a11y impact, and welcomed for non-breaking changes when relevant.

@lokesh-sagi125 lokesh-sagi125 marked this pull request as ready for review October 11, 2024 19:51
@lokesh-sagi125
Copy link
Contributor Author

hey @akolson @MisRob can you review this code for me , i have attached the before and after videos.i have run lint-fix multiple times and tried adding and removing an extra line but couldn't fix it .

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.

2 participants