Skip to content
This repository has been archived by the owner on Feb 25, 2023. It is now read-only.

Settings refactor #541

Merged
merged 8 commits into from
May 24, 2020

Conversation

toasted-nutbread
Copy link
Collaborator

Cleans up a few things in preparation for some larger refactoring and changes.

  • Debug info block is removed since the same data and more can be found by using the backup system.
  • Updated some of the controls for options with special handling. This includes some of the Anki settings and general.enableClipboardPopups handling.
  • This also fixes a bug where the Anki card fields would not be updated when changing the modifying profile.

@toasted-nutbread toasted-nutbread mentioned this pull request May 17, 2020
Copy link
Collaborator

@siikamiika siikamiika left a comment

Choose a reason for hiding this comment

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

There's still a related issue with Anki template preview that I noticed when doing #539. If you change result grouping mode, the object used to generate the preview isn't updated before you reload the settings page or edit the preview text. Maybe not in scope for this PR unless it reveals other issues that should be fixed.

@@ -265,6 +250,9 @@ async function onOptionsUpdated({source}) {

const optionsContext = getOptionsContext();
const options = await getOptionsMutable(optionsContext);

onAnkiOptionsChanged();
Copy link
Collaborator

Choose a reason for hiding this comment

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

await

@@ -260,6 +259,7 @@ async function onOptionsUpdated({source}) {
const options = await getOptionsMutable(optionsContext);

document.querySelector('#enable-clipboard-popups').checked = options.general.enableClipboardPopups;
ankiTemplatesUpdateValue();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does this not await on purpose?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I believe all the awaits are omitted because they are unnecessary. The anki settings are modified independently, so there is no reason to await.

@toasted-nutbread
Copy link
Collaborator Author

There's still a related issue with Anki template preview that I noticed when doing #539. If you change result grouping mode, the object used to generate the preview isn't updated before you reload the settings page or edit the preview text. Maybe not in scope for this PR unless it reveals other issues that should be fixed.

In general, most of the files in the settings folder need to be refactored. I am trying to do incremental changes to get to that point. The layout/design changes for the updated settings page also necessitate some changes. So tasks like that will probably be addressed during those changes.

@toasted-nutbread toasted-nutbread merged commit 3089bb7 into FooSoft:master May 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants