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.

Sign up for GitHub

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

Jump to bottom

Fix changing filters not resetting multiselected media cards #5377

Merged
merged 1 commit into from
Jul 12, 2024
Merged

Fix changing filters not resetting multiselected media cards #5377

merged 1 commit into from
Jul 12, 2024

Conversation

ConnorS1110
Copy link
Contributor

Changes
Multi-selected options are now cleared by calling hideSelections from the multiSelect component when a change is triggered in the filterdialog component. hideSelections is also called in the list controller when filters have been changed and there actually filters being applied instead of removed.

Issues
Fixes #5209

@ConnorS1110 ConnorS1110 requested a review from a team as a code owner April 7, 2024 16:22
@ConnorS1110 ConnorS1110 force-pushed the fix-multiselect-filter branch 2 times, most recently from a5fa2cb to fdce2e2 Compare April 7, 2024 16:39
@sonarcloud SonarCloud

This comment was marked as outdated.

Sign in to view
@jellyfin-bot jellyfin-bot added the merge conflict Conflicts prevent merging label May 7, 2024
@jellyfin-bot

This comment has been minimized.

Sign in to view
@thornbill thornbill added the bug Something isn't working label May 18, 2024
@thornbill thornbill added this to the v10.10.0 milestone May 18, 2024
@thornbill
Copy link
Member

@ConnorS1110 could you resolve the conflicts here? Now that 10.9 is finally out let's work on getting this in. 👍

@jellyfin-bot jellyfin-bot removed the merge conflict Conflicts prevent merging label May 19, 2024
@ConnorS1110
Copy link
Contributor Author

@thornbill fixed the merge conflict and looks like everything is still good

@jellyfin-bot
Copy link
Collaborator

Cloudflare Pages deployment

Latest commit b2dbae0
Status ✅ Deployed!
Preview URL https://5db16d67.jellyfin-web.pages.dev
Type 🔀 Preview

View build logs
View bot logs

@thornbill
Copy link
Member

Sorry got the delay. I’ve been a bit busy dealing with bugs in 10.9 but I think that is finally starting to settle down.

This would probably be a good candidate for 10.9 if you don’t mind going through the rebase process.

@ConnorS1110
Copy link
Contributor Author

Do you mean a candidate for 10.10? @thornbill

@thornbill
Copy link
Member

No I mean 10.9 since it is a relatively small bug fix we can get it in a patch release. You can follow the process described here to update this PR to target the release branch: #5680 (comment)

@jellyfin-bot

This comment was marked as outdated.

Sign in to view
@jellyfin-bot jellyfin-bot added the merge conflict Conflicts prevent merging label Jun 9, 2024
@ConnorS1110 ConnorS1110 changed the base branch from master to release-10.9.z June 9, 2024 16:52
@ConnorS1110
Copy link
Contributor Author

I don't think I've done this correctly, but I can't figure out what I did wrong. I rebased and accepted the current changes for everything.

@jellyfin-bot jellyfin-bot removed the merge conflict Conflicts prevent merging label Jun 9, 2024
@ConnorS1110
Copy link
Contributor Author

Ok I think? I fixed it. I am assuming I was supposed to literally remove all the commit lines that were not mine when doing the rebase

@dmitrylyzo
Copy link
Contributor

dmitrylyzo commented Jun 9, 2024 •
edited
Loading

I don't think I've done this correctly, but I can't figure out what I did wrong. I rebased and accepted the current changes for everything.

It just grabbed all commits to your branch.

If you are using Git CLI, use interactive mode (-i) and remove all but your commit from the "scenario".

Some GUI tools can do interactive rebasing too, but I don't use any of them.

UPD:
I am slow writer 🐢 😅

@thornbill thornbill modified the milestones: v10.10.0, v10.9.7 Jun 10, 2024
thornbill
thornbill reviewed Jun 10, 2024
View reviewed changes
Copy link
Member

@thornbill thornbill left a comment

Choose a reason for hiding this comment

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

Just a minor note, we are somewhat inconsistent with spacing after a function name, but we should prefer no space.

src/components/filterdialog/filterdialog.js Outdated Show resolved Hide resolved
src/controllers/list.js Outdated Show resolved Hide resolved
@sonarcloud SonarCloud
Copy link

sonarcloud bot commented Jun 10, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

@thornbill thornbill modified the milestones: v10.9.7, v10.9.8 Jun 25, 2024
@thornbill thornbill added the stable backport Backport into the next stable release label Jul 10, 2024
@thornbill thornbill merged commit 798b408 into jellyfin:release-10.9.z Jul 12, 2024
11 checks passed
@jellyfin-bot jellyfin-bot removed the stable backport Backport into the next stable release label Jul 21, 2024
joshuaboniface pushed a commit that referenced this pull request Jul 21, 2024
@ConnorS1110 @thornbill
Backport pull request #5377 from jellyfin-web/release-10.9.z
cf6eaa6 
Fix changing filters not resetting multiselected media cards

Original-merge: 798b408

Merged-by: thornbill <thornbill@users.noreply.github.com>

Backported-by: Bill Thornton <thornbill@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@thornbill thornbill thornbill approved these changes

Assignees
No one assigned
Labels
bug Something isn't working
Projects
Release 10.9.z
Status: Done
Milestone
v10.9.8
Development

Successfully merging this pull request may close these issues.

Changing filters while in selection mode, doesn't cancel selection mode completely
4 participants
@ConnorS1110 @jellyfin-bot @thornbill @dmitrylyzo