Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # 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

# for free to join this conversation on GitHub. Already have an account? # 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