Add KFocusTrap and use it in KModal

[lokesh-sagi125]

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: copy FocusTrap from kolibri into KFocusTrap in the KDS and use it in the KModal
• Products impact: -
• Addresses: Add KFocusTrap and use it in KModal #746
• Components: KModal , KFocuaTrap
• Breaking: no
• Impacts a11y: -
• Guidance: -

[lokesh-sagi125]

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 .

[lokesh-sagi125]

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

[MisRob]

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.

[MisRob]

@akolson @AlexVelezLl I am linking the previous PR with review #764 that this PR is expected to have resolved, notably #764 (comment)

[MisRob]

Related Kolibri PR learningequality/kolibri#12718

[MisRob]

@AlexVelezLl feel free to review if you wish, I just removed you because I wanted to align reviewers with the related Kolibri PR #799 and I think you're already taking care of another reviews :)

[MisRob]

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

[akolson]

The description for the shouldFocusFirstEl event is not showing up in the docs. Please move it inside the function just above this.$emit(...)

[akolson]

The description for the shouldFocusLastEl event is not showing up in the docs. Please move it inside the function just above this.$emit(...)

[akolson]

This should be the below so that the description for the slot is added in the component's documentation. I've added @slot.

    <!-- @slot Default slot where the focusable content will be rendered -->

[akolson]

The code changes look correct to me. I have left some comments to fix some issues with the documentation. Thanks @lokesh-sagi125

[akolson]

@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>"

[lokesh-sagi125]

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 ' .

[akolson]

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?

[lokesh-sagi125]

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

[akolson]

@lokesh-sagi125 reformat but don't run the lint command and let's see what happens

[AlexVelezLl]

Just higlighted the needed changes to fix the linting issues :). Our current linter gets a bit confused when there aren't two lines between the main template, script and style blocks.

[lokesh-sagi125]

hey @AlexVelezLl i did commit the changes you suggested and it seems to have solved the linting issue.

[lokesh-sagi125]

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?

[akolson]

LGTM! Thanks @lokesh-sagi125. @AlexVelezLl, please feel free to merge whenever you are ready!

[AlexVelezLl]

Hey @lokesh-sagi125! You can send an email to [email protected] so we can send you the invitation to our slack channel 🤗. Thank you!!