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

Maintenance tasks #558

Merged
merged 7 commits into from
Feb 27, 2024
Merged

Maintenance tasks #558

merged 7 commits into from
Feb 27, 2024

Conversation

MisRob
Copy link
Member

@MisRob MisRob commented Feb 23, 2024

Description

A collection of maintenance tasks. See commit messages or changelog items below. Composables location clean up was discussed in the meeting few weeks ago.

@rtibbles Could you please check package.json updates that we talked about (browserslist, pinned dependencies)?

Changelog

  • #558
    • Description: Move useKResponsiveWindow from /lib to /lib/composables
    • Products impact: Location update
    • Addresses: -
    • Components: useKResponsiveWindow
    • Breaking: yes
    • Impacts a11y: -
    • Guidance: Update import useKResponsiveWindow from 'kolibri-design-system/lib/useKResponsiveWindow'; from import useKResponsiveWindow from 'kolibri-design-system/lib/composables/useKResponsiveWindow';
  • #558
    • Description: Remove deprecated KResponsiveWindow's mixin documentation page in favor of a new useKResponsiveWindow page
    • Products impact: none
    • Addresses: -
    • Components: KResponsiveWindow
    • Breaking: -
    • Impacts a11y: -
    • Guidance: -
  • #558
    • Description: Adds engines and browserlist to package.json. Pins dependencies to exact version.
    • Products impact: Dependencies
    • Addresses: -
    • Components: -
    • Breaking: -
    • Impacts a11y: -
    • Guidance: -
  • #558
    • Description: Internal maintenance tasks: extract common logic to utils, move private composables to /lib/composables and indicate that they are private by _ prefix in their filename. dev docs updates.
    • Products impact: none
    • Addresses: -
    • Components: -
    • Breaking: -
    • Impacts a11y: -
    • Guidance: -

Steps to test

Testing checklist

  • Contributor has fully tested the PR manually
  • If there are any front-end changes, before/after screenshots are included
  • Critical and brittle code paths are covered by unit tests
  • The change is described in the changelog section above

Reviewer guidance

  • Is the code clean and well-commented?
  • Are there tests for this change?
  • Are all UI components LTR and RTL compliant (if applicable)?
  • Add other things to check for here

After review

  • The changelog item has been pasted to the CHANGELOG.md

- Move all composables to lib/composables
- Indicate private composables by "_" prefix
- Remove deprecated KResponsiveWindow mixin documentation page
  in favor of a new useKResponsiveWindow page
These are installed together with KDS
in products so this ensure we know exactly
what versions are used.
@MisRob MisRob marked this pull request as ready for review February 23, 2024 12:45
@MisRob MisRob added the TODO: needs review Waiting for review label Feb 23, 2024
@MisRob MisRob requested a review from akolson February 23, 2024 12:48
Copy link
Member

@nucleogenesis nucleogenesis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes make sense to me thanks Misha!

Copy link
Member

@akolson akolson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes look correct to me. Thanks @MisRob

Copy link
Member

@AlexVelezLl AlexVelezLl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! The only question that would come to mind would be. Why does the useKDimensions have to be private? It seems to me that it could be useful in some contexts if you want to watch the windows height and width.

@MisRob
Copy link
Member Author

MisRob commented Feb 27, 2024

@AlexVelezLl

The only question that would come to mind would be. Why does the useKDimensions have to be private? It seems to me that it could be useful in some contexts if you want to watch the windows height and width.

In the context of this PR, it's private because it was private before, it just wasn't indicated in its name.

You're right it could be useful. If it's needed in the products, we can prepare a documentation page for it and make it public.

@MisRob MisRob merged commit 0d4aa5e into learningequality:develop Feb 27, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
TODO: needs review Waiting for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants