Add KFocusTrap and use it in KModal
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.
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 .
hey @MisRob i added the documentation page but i cant find a way to add info to the props as it was auto-generated i tried adding comments in the main code but it still didint show up in the documentation page , how can i add info to these parts of the documentation page
Hi @lokesh-sagi125, thank you.
i added the documentation page but i cant find a way to add info to the props as it was auto-generated i tried adding comments in the main code but it still didint show up in the documentation page
Running the development server with yarn dev should auto-generate the documentation.
I see props documented properly, but events and slots are missing. Please inspect the KImg component source code ( example 1, example 2) and its auto-generated documentation for slots and events. That should guide you. If it still doesn't work, message us again. Note that I am soon taking time off till the end of October, so best to mention @akolson or @AlexVelezLl.
@akolson @AlexVelezLl I am linking the previous PR with review https://github.com/learningequality/kolibri-design-system/pull/764 that this PR is expected to have resolved, notably https://github.com/learningequality/kolibri-design-system/pull/764#discussion_r1741697060
Related Kolibri PR https://github.com/learningequality/kolibri/pull/12718
@AlexVelezLl feel free to review if you wish, I just removed you because I wanted to align reviewers with the related Kolibri PR https://github.com/learningequality/kolibri-design-system/pull/799 and I think you're already taking care of another reviews :)
Regarding releases, this won't be a breaking change, however a bit of planning as of when to merge it, depending on the state of the Kolibri PR, may be needed - cc @AlexVelezLl
@lokesh-sagi125 also please remember to run the below before you commit your changes so that the linting issues are also resolved. Thanks
yarn lint-fix
git add .
git commit -m "<your commit message here>"
hey @akolson i have made changes to the 'KFocusTrap' component according to your guidance and all the functions are documented now
and i have tried to fix the linting issue as you told but it seems to persist, maybe due to the code being re-written after 'yarn lint-fix ' .
Hi @lokesh-sagi125, the linting issues are particularly reported in the below;
Kolibri Linter: Linting errors for lib/KFocusTrap.vue
Kolibri Linter: L14 |
Kolibri Linter: ^ Please use space for indentation and keep 2 length. (space-tab-mixed-disabled)
Kolibri Linter: L28 |<script>
Kolibri Linter: ^ Need two endlines between top-level tags. (--vue-component-conventions)
Kolibri Linter: L107 |<style scoped>
Kolibri Linter: ^ Need two endlines between top-level tags. (--vue-component-conventions)
Maybe manually adding the endlines and space as advised above could help?
yes @akolson i did try manually adding the two endlines and the indentation , but it gets formatted once i run 'yarn lint-fix' or run the devserver
@lokesh-sagi125 reformat but don't run the lint command and let's see what happens
hey @AlexVelezLl i did commit the changes you suggested and it seems to have solved the linting issue.
hi @AlexVelezLl i have come across this article , https://medium.com/learning-equality/gsoc-24-my-journey-as-a-contributor-at-learning-equality-4ed4c623b189 and was aiming to go down a similar path , how can i join the slack channel? are there any pre-requisites?
Hey @lokesh-sagi125! You can send an email to [email protected] so we can send you the invitation to our slack channel :hugs:. Thank you!!