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

Upgrade to KDS v3.0.0 and reference npm package #11873

Merged
merged 1 commit into from
Feb 16, 2024

Conversation

MisRob
Copy link
Member

@MisRob MisRob commented Feb 14, 2024

Summary

  • Upgrades KDS from somewhere in the middle of v3.0.0 commits to v3.0.0. Includes Kolibri code updates related to the breaking change in regards to value --> buttonValue renaming. See KDS v3.0.0 release notes. I've manually tested a few places with radio buttons after the update but not all of them.

  • Uses npm package to install KDS

    • I discussed with @rtibbles that it doesn't matter that much if we pin minor or patch because of the dependabot workflow, so we went with patch ~. This has a benefit of knowing exactly what version is installed.

Reviewer guidance

  • value --> buttonValue
    • You could check that for all radio buttons I updated, the intention for using value was indeed to use it as a prop to set the radio button's value HTML attribute (rather than as Vue value binding - in such cases it should stay :value)
    • Are there some forgotten places that still need to be updated?
    • You could test manually few places with radio buttons

Testing checklist

  • Contributor has fully partially :) tested the PR manually
  • If there are any front-end changes, before/after screenshots are included
  • Critical user journeys are covered by Gherkin stories
  • Critical and brittle code paths are covered by unit tests

PR process

  • PR has the correct target branch and milestone
  • PR has 'needs review' or 'work-in-progress' label
  • If PR is ready for review, a reviewer has been added. (Don't use 'Assignees')
  • If this is an important user-facing change, PR or related issue has a 'changelog' label
  • If this includes an internal dependency change, a link to the diff is provided

Reviewer checklist

  • Automated test coverage is satisfactory
  • PR is fully functional
  • PR has been tested for accessibility regressions
  • External dependency files were updated if necessary (yarn and pip)
  • Documentation is updated
  • Contributor is in AUTHORS.md

@github-actions github-actions bot added DEV: renderers HTML5 apps, videos, exercises, etc. APP: Device Re: Device App (content import/export, facility-syncing, user permissions, etc.) APP: Facility Re: Facility App (user/class management, facility settings, csv import/export, etc.) APP: Coach Re: Coach App (lessons, quizzes, groups, reports, etc.) APP: User Re: User app (sign-in, sign-up, user profile, etc.) APP: Setup Wizard Re: Setup Wizard (facility import, superuser creation, settings, etc.) DEV: frontend labels Feb 14, 2024
- Upgrades KDS from somewhere in the middle of v3.0.0 commits
to v3.0.0. Includes Kolibri code updates related to the breaking
change in regards to value --> buttonValue renaming. See KDS
v3.0.0 release notes
https://github.com/learningequality/kolibri-design-system/releases/tag/v3.0.0

- Uses npm package to install KDS
@MisRob MisRob added the TODO: needs review Waiting for review label Feb 14, 2024
@MisRob MisRob marked this pull request as ready for review February 14, 2024 09:01
@MisRob MisRob requested a review from pcenov February 16, 2024 04:48
@MisRob
Copy link
Member Author

MisRob commented Feb 16, 2024

Hi @pcenov, would you have a moment to confirm that radio buttons at a few (any) places in Kolibri still work well?

Copy link
Member

@pcenov pcenov left a comment

Choose a reason for hiding this comment

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

Hi @MisRob - no issues observed with the radio buttons in the setup wizard and anywhere else!

Copy link
Member

@LianaHarris360 LianaHarris360 left a comment

Choose a reason for hiding this comment

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

I did not find any places that still needed changes, everything looks good and works as expected!

@MisRob MisRob merged commit 4f12ce5 into learningequality:develop Feb 16, 2024
31 checks passed
@MisRob
Copy link
Member Author

MisRob commented Feb 16, 2024

Thanks both

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
APP: Coach Re: Coach App (lessons, quizzes, groups, reports, etc.) APP: Device Re: Device App (content import/export, facility-syncing, user permissions, etc.) APP: Facility Re: Facility App (user/class management, facility settings, csv import/export, etc.) APP: Setup Wizard Re: Setup Wizard (facility import, superuser creation, settings, etc.) APP: User Re: User app (sign-in, sign-up, user profile, etc.) DEV: frontend DEV: renderers HTML5 apps, videos, exercises, etc. SIZE: medium TODO: needs review Waiting for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants