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

Replace KResponsiveWindow mixin by useKResponsiveWindow composable #11321

Closed
13 tasks done
MisRob opened this issue Sep 29, 2023 · 6 comments · Fixed by #11534
Closed
13 tasks done

Replace KResponsiveWindow mixin by useKResponsiveWindow composable #11321

MisRob opened this issue Sep 29, 2023 · 6 comments · Fixed by #11534
Labels
DEV: frontend P2 - normal Priority: Nice to have

Comments

@MisRob
Copy link
Member

MisRob commented Sep 29, 2023

Summary

We extensively use KResponsiveWindow (provides reactive window's size information) as mixin in Kolibri. The latest version of Kolibri Design System (KDS) installed on the Kolibri's develop branch offers better alternative, useKResponsiveWindow composable. This issue encapsulates a group of issues whose common goal is to completely get rid of KResponsiveWindow mixin in favor of using useKResponsiveWindow composable in the whole Kolibri.

Note that the ways we import the mixin in our files are not always consistent, for example you may see:

  • import responsiveWindowMixin from 'kolibri.coreVue.mixins.responsiveWindowMixin';
  • import KResponsiveWindowMixin from 'kolibri-design-system/lib/KResponsiveWindowMixin';

You can use Vue Devtools and codebase search to find out pages that need to be previewed in the app based on the components/files you're modifying. Please test your changes before opening a pull request (see acceptance criteria).

Acceptance criteria (for each one of the issues listed below)

  • useKResponsiveWindow composable is used instead of KResponsiveWindow mixin in an area specified by a sub-issue
  • All occurences of properties provided by KResponsiveWindow (windowIsSmall, windowBreakpoint, etc., see the full list here) are imported from useKResponsiveWindow composable in a file where KResponsiveWindow mixin is being removed
  • Whatever logic was KResponsiveWindow mixin used for (for example, using different styling for mobile devices) is manually tested and still works as expected with useKResponsiveWindow composable
  • There is a list of affected pages in a pull request description (e.g. their URLs)

References

Sub-issues

Sub-issues

Preview Give feedback
  1. DEV: frontend P2 - normal good first issue help wanted
    thesujai
  2. DEV: frontend P2 - normal good first issue help wanted
    andreamisuraca
  3. DEV: frontend P2 - normal good first issue help wanted
    thesujai
  4. DEV: frontend P2 - normal good first issue help wanted
    ShivangRawat30
  5. DEV: frontend P2 - normal good first issue help wanted
    thesujai
  6. DEV: frontend P2 - normal good first issue help wanted
    FireSuperior482
  7. DEV: frontend P2 - normal good first issue help wanted
    ShivangRawat30
  8. DEV: frontend P2 - normal good first issue help wanted
    ShivangRawat30
  9. DEV: frontend P2 - normal good first issue help wanted
    uraj323
  10. DEV: frontend P2 - normal good first issue help wanted
    moweiss
  11. DEV: frontend P2 - normal good first issue help wanted
    ShivangRawat30
  12. DEV: frontend P2 - normal good first issue help wanted
    thesujai
  13. DEV: frontend P2 - normal good first issue help wanted
    a6ar55
@MisRob
Copy link
Member Author

MisRob commented Sep 29, 2023

Note that the current version of useKResponsiveWindow might be buggy on some occasions, e.g. #11212 is caused by it. This will be fixed soon learningequality/kolibri-design-system#453

@MisRob
Copy link
Member Author

MisRob commented Sep 29, 2023

If you'd like to contribute, please don't ask for an assignment of this issue, but rather in a sub-issue.

@MisRob MisRob added P2 - normal Priority: Nice to have DEV: frontend labels Sep 29, 2023
@thesujai
Copy link
Contributor

thesujai commented Oct 8, 2023

Hey @MisRob, I was participating in HactoberFest.
Would it be possible for you to add a hacktoberfest-accepted label in my PRs for the sub-issues. I would be really thankful to you. As they asked we can ask the maintainers to add just the label in PR if they aren't participating in the fest.
Here is the link: https://hacktoberfest.com/participation/#maintainers
Its fine if for some reasons you aren't able to do so:)

@MisRob
Copy link
Member Author

MisRob commented Oct 10, 2023

Hello @thesujai, I will reach out back in a few days in regards to Hactoberfest

@thesujai
Copy link
Contributor

Okay @MisRob

@MisRob
Copy link
Member Author

MisRob commented Oct 12, 2023

@thesujai Answered here #11356 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DEV: frontend P2 - normal Priority: Nice to have
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants