kolibri-design-system icon indicating copy to clipboard operation
kolibri-design-system copied to clipboard

Add KFocusTrap and use it in KModal

Open lokesh-sagi125 opened this issue 1 year ago • 1 comments

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 avatar Oct 11 '24 19:10 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 avatar Oct 11 '24 20:10 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 Uploading Screenshot 2024-10-14 at 11.16.53 AM.png…

lokesh-sagi125 avatar Oct 14 '24 05:10 lokesh-sagi125

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 avatar Oct 14 '24 08:10 MisRob

@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

MisRob avatar Oct 14 '24 08:10 MisRob

Related Kolibri PR https://github.com/learningequality/kolibri/pull/12718

MisRob avatar Oct 14 '24 09:10 MisRob

@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 :)

MisRob avatar Oct 16 '24 07:10 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

MisRob avatar Oct 16 '24 07:10 MisRob

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

akolson avatar Oct 21 '24 13:10 akolson

hey @akolson i have made changes to the 'KFocusTrap' component according to your guidance and all the functions are documented now Screenshot 2024-10-22 at 5 39 52 PM 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 ' .

lokesh-sagi125 avatar Oct 22 '24 12:10 lokesh-sagi125

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?

akolson avatar Oct 22 '24 13:10 akolson

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 avatar Oct 24 '24 11:10 lokesh-sagi125

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

akolson avatar Oct 24 '24 11:10 akolson

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

lokesh-sagi125 avatar Oct 24 '24 13:10 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?

lokesh-sagi125 avatar Oct 24 '24 13:10 lokesh-sagi125

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!!

AlexVelezLl avatar Oct 28 '24 17:10 AlexVelezLl