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

feat: Make suspension settings into per-deck options #779

Merged
merged 117 commits into from
Oct 20, 2023

Conversation

RisingOrange
Copy link
Collaborator

@RisingOrange RisingOrange commented Oct 18, 2023

Make the card suspension settings into per-deck options. (Currently they are global options for all deck).

Related issues

Make the card suspension settings per-deck, add them to the new Deck Management dialog

Proposed changes

  • Move the two options related to suspending cards from the general configuration dialog to the new decks dialog.
  • Change the logic for suspending cards in the importer to use the per-deck option, instead of the global one.
  • Update usages of AnkiHubImporter.import_ankihub_deck to pass in the new arguments for the suspension options.

How to reproduce

On the main Anki window, open the AnkiHub menu and click on Deck Management to reach the dialog shown below.

Screenshots and videos

Copy link
Collaborator

@pedroven pedroven left a comment

Choose a reason for hiding this comment

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

I noticed some issues when I was testing this PR:

  • if I unsubscribe from the addon the deck will not appear anymore in the list of subscribed decks even if I subscribe to the same deck again, is that intended? Update: After I click in sync, the deck appeared again
  • The default option is set to "Always" instead of ”Suspend new cards of new notes”
  • When I change the option from "Always" to "Never" and then open "Deck Management" again the option selected remains as "Always"

@RisingOrange
Copy link
Collaborator Author

RisingOrange commented Oct 19, 2023

@pedroven

if I unsubscribe from the addon the deck will not appear anymore in the list of subscribed decks even if I subscribe to the same deck again, is that intended? Update: After I click in sync, the deck appeared again

This is because only installed decks are showed in the list in the Deck Management Dialog now. Previously we were showing all subscribed decks, even if they are not installed. You could sync with AnkiHub to install the deck again and then it would show up in the list.
I agree that this is a bit confusing, because it says Subscribed AnkiHub Decks, but it doesn't really show all subscribed decks. I will discuss with the product team how to solve this, because we don't want to show options for decks that are not installed.

The default option is set to "Always" instead of ”Suspend new cards of new notes”

When I change the option from "Always" to "Never" and then open "Deck Management" again the option selected remains as "Always"

Good catch! I fixed this now in 289178e.

@RisingOrange RisingOrange requested a review from pedroven October 19, 2023 21:07
pedroven
pedroven previously approved these changes Oct 19, 2023
Base automatically changed from feat/redesign-decks-dialog to main October 20, 2023 09:23
@RisingOrange RisingOrange dismissed pedroven’s stale review October 20, 2023 09:23

The base branch was changed.

@RisingOrange RisingOrange force-pushed the feat/make-suspension-settings-per-deck branch from 759d610 to efd9a02 Compare October 20, 2023 10:42
@RisingOrange RisingOrange requested a review from pedroven October 20, 2023 11:03
@RisingOrange
Copy link
Collaborator Author

@pedroven Since the last review I merged in the refactor of the ui setup of the dialog from #777 and refactored the setup of the the suspension options as well to make it more readable.

@RisingOrange RisingOrange merged commit b82d5c2 into main Oct 20, 2023
12 checks passed
@RisingOrange RisingOrange deleted the feat/make-suspension-settings-per-deck branch October 20, 2023 12:49
@RisingOrange RisingOrange restored the feat/make-suspension-settings-per-deck branch October 20, 2023 12:55
@RisingOrange RisingOrange deleted the feat/make-suspension-settings-per-deck branch October 20, 2023 12:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants