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

Use KRadioButtonGroup in the language switcher modal and on the Device settings page #12325

Conversation

muditchoudhary
Copy link
Contributor

@muditchoudhary muditchoudhary commented Jun 20, 2024

Summary

References

Reviewer guidance


Testing checklist

  • Contributor has fully 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 APP: Device Re: Device App (content import/export, facility-syncing, user permissions, etc.) DEV: frontend SIZE: medium labels Jun 20, 2024
@muditchoudhary
Copy link
Contributor Author

Hi @pcenov @radinamatic could you please QA test now? I fixed the tab issue on the change language modal.

cc: @MisRob

@MisRob
Copy link
Member

MisRob commented Jun 20, 2024

Thanks @muditchoudhary!

For QA, test instructions are written here #12243

@pcenov
Copy link
Member

pcenov commented Jun 21, 2024

Thanks a lot @muditchoudhary!
@radinamatic I confirm that tab navigation through the radio buttons in the language modal is working correctly in Firefox:

2024-06-21_13-32-03.mp4

@muditchoudhary
Copy link
Contributor Author

Thank you @pcenov

@MisRob
Copy link
Member

MisRob commented Aug 16, 2024

@radinamatic @pcenov Could you please re-test this PR? I've tried briefly and looked good to me.

@MisRob
Copy link
Member

MisRob commented Aug 16, 2024

Actually we will need one more code adjustment, so let's wait for final version before QA. I will let you know when it's ready @radinamatic @pcenov

@MisRob
Copy link
Member

MisRob commented Aug 19, 2024

Thanks for getting everything ready here @muditchoudhary! @radinamatic @pcenov it's ready for final QA :)

@pcenov
Copy link
Member

pcenov commented Aug 23, 2024

Hi @MisRob - no issues observed while manually testing - good to go!

@MisRob
Copy link
Member

MisRob commented Aug 26, 2024

Thanks! We merged the KDS PR. As soon as the new KDS release is ready, I will install it here and we can merge.

@MisRob MisRob changed the title DO_NOT_MERGE_ONLY_FOR_QA KRadioButtonGroup testing [DO NOT MERGE UNTIL KDS RELEASE] KRadioButtonGroup testing Aug 26, 2024
@MisRob MisRob changed the title [DO NOT MERGE UNTIL KDS RELEASE] KRadioButtonGroup testing [DO NOT MERGE UNTIL KDS RELEASE] Use KRadioButtonGroup in the language switcher modal and on the Device settings page Aug 26, 2024
@MisRob MisRob marked this pull request as ready for review August 26, 2024 09:31
@MisRob MisRob changed the title [DO NOT MERGE UNTIL KDS RELEASE] Use KRadioButtonGroup in the language switcher modal and on the Device settings page Use KRadioButtonGroup in the language switcher modal and on the Device settings page Sep 4, 2024
@MisRob MisRob force-pushed the KRadioButtonGroup_for_change_language_modal branch from 554d90f to a4fe44b Compare September 4, 2024 19:37
@MisRob MisRob force-pushed the KRadioButtonGroup_for_change_language_modal branch from a4fe44b to bbec768 Compare September 4, 2024 19:39
Copy link
Member

@MisRob MisRob left a comment

Choose a reason for hiding this comment

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

QA passed and code also looks well to me. I rebased on top of the latest develop that contains the KDS release with KRadioButtonGroup. Let me preview the latest build as soon as it's ready and then we can merge.

@MisRob
Copy link
Member

MisRob commented Sep 4, 2024

Briefly re-tested on the most recent build, all good

@MisRob
Copy link
Member

MisRob commented Sep 4, 2024

Yey @muditchoudhary, it's live now!

@MisRob MisRob merged commit 8fbc77c into learningequality:develop Sep 4, 2024
40 checks passed
@muditchoudhary
Copy link
Contributor Author

Yayy!! Thank you for telling me @MisRob

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
APP: Device Re: Device App (content import/export, facility-syncing, user permissions, etc.) DEV: frontend SIZE: medium
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants