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

Resolves KCheckbox issues selecting all items in trash modal #4526

Merged
merged 4 commits into from
May 29, 2024

Conversation

joeysantia
Copy link
Contributor

@joeysantia joeysantia commented Apr 19, 2024

Summary

Description of the change(s) you made

Changed the @change property in TrashModal.vue to @input and changed the value of the checkbox to reflect whether all items are selected (e.g. value is falsy Boolean if not all items are selected). Edited the TrashModal tests to reflect this implementation

Manual verification steps performed

  1. Edited the value of the selectall Checkbox
  2. Changed @change to @input
  3. Ran tests
  4. Edited the trashModal.spec.js file to reflect @input property
  5. Re-ran and passed tests

Reviewer guidance

How can a reviewer test these changes?

  1. Manually use selectall button on the trash page
  2. Run trashModal.spec.js tests

Contributor's Checklist

Testing:

  • Code is clean and well-commented
  • 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
  • Any new interactions have been added to the QA Sheet
  • Critical and brittle code paths are covered by unit tests

Reviewer's Checklist

This section is for reviewers to fill out.

  • 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

@marcellamaki
Copy link
Member

This is related to #4510

@bjester bjester self-assigned this Apr 23, 2024
@bjester bjester changed the title Selectall Resolves KCheckbox issues selecting all items in trash modal Apr 23, 2024
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.

Hi @joeysantia 👋 Apologies for the delay in following up on this PR. The change seems to be correct, I just left a note about a specific case we can fix, and after that it should be good to go 👐.

@@ -23,10 +23,10 @@
<VLayout v-if="props.header.selectAll" row align-center>
<VFlex shrink>
<Checkbox
:value="Boolean(selected.length)"
:value="selected.length === items.length"
Copy link
Member

Choose a reason for hiding this comment

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

To explicitly pass the boolean checkbox value, we need to use the prop inputValue instead. Without this, we have this issue: when all items are selected, the checkbox doesnt appear as selected.

Compartir.pantalla.-.2024-05-17.07_37_58.mp4

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah I see ! I'll make that change

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.

Thank you! @joeysantia LGTM! It seems to be fixed now 🎉.

@AlexVelezLl AlexVelezLl merged commit fa1892b into learningequality:unstable May 29, 2024
13 checks passed
@akolson akolson mentioned this pull request Aug 13, 2024
@akolson akolson mentioned this pull request Oct 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants