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

Fix torrent content checkbox state under certain conditions #22190

Merged
merged 1 commit into from
Jan 24, 2025

Conversation

thalieht
Copy link
Contributor

@thalieht thalieht commented Jan 21, 2025

Set folder as partially checked when it has at least one partially checked child and none unchecked.

Closes #22189.

Set folder as partially checked when it has at least one
partially checked child and none unchecked.

Closes qbittorrent#22189
@thalieht thalieht added the GUI GUI-related issues/changes label Jan 21, 2025
@glassez
Copy link
Member

glassez commented Jan 21, 2025

Set folder as partially checked when it has at least one partially checked child and none unchecked.

Should't it behave as the following?

  1. Folder is checked if all its direct children are checked.
  2. Folder is unchecked if all its direct children are unchecked.
  3. Folder is partially checked otherwise.

@thalieht
Copy link
Contributor Author

Set folder as partially checked when it has at least one partially checked child and none unchecked.

Should't it behave as the following?

It does... doesn't it? Or did i phrase it too weirdly? I tried to describe the conditions for #22189.

@glassez
Copy link
Member

glassez commented Jan 22, 2025

Set folder as partially checked when it has at least one partially checked child and none unchecked.

Should't it behave as the following?

It does... doesn't it? Or did i phrase it too weirdly? I tried to describe the conditions for #22189.

I didn't have time to look at anything except the PR description. I expected it to describe how qBittorrent behaves after PR is merged. The described behavior looks incorrect, so I have provided my suggestion for correct behavior.

Copy link
Member

@glassez glassez left a comment

Choose a reason for hiding this comment

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

Code changes LGTM. Didn't test.

@glassez glassez added this to the 5.1 milestone Jan 22, 2025
@glassez
Copy link
Member

glassez commented Jan 22, 2025

Set folder as partially checked when it has at least one partially checked child and none unchecked.

Should't it behave as the following?

It does... doesn't it?

Judging by code changes it does.

@glassez glassez merged commit 05787d9 into qbittorrent:master Jan 24, 2025
14 checks passed
@glassez
Copy link
Member

glassez commented Jan 24, 2025

@thalieht
Thank you!

@thalieht thalieht deleted the prioCheckbox branch January 24, 2025 14:47
@Chocobo1
Copy link
Member

Is v5.0.x affected? If yes, then it should be backported.

@thalieht
Copy link
Contributor Author

Is v5.0.x affected?

Yes.

@Chocobo1
Copy link
Member

@thalieht
Do you mind submit a backport PR?

@glassez
Copy link
Member

glassez commented Jan 26, 2025

@thalieht
Do you mind submit a backport PR?

I would stick to the previous arrangement to do backports via single cumulative "backport PR" (whenever possible).
Or is it irrelevant now for some reason?
@sledgehammer999?

@glassez glassez modified the milestones: 5.1, 5.0.4 Jan 26, 2025
glassez pushed a commit to glassez/qBittorrent that referenced this pull request Jan 26, 2025
@glassez
Copy link
Member

glassez commented Jan 26, 2025

Backported by #22207.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
GUI GUI-related issues/changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

content's checkbox: check, partial or empty logic
4 participants