Skip to content

Commit

Permalink
Consistently dispatch events, if needed, when setting AppOptions
Browse files Browse the repository at this point in the history
Currently we'll only dispatch events, for the options that support it, when updating preferences. Since this could potentially lead to inconsistent behaviour, let's avoid any future surprises by *always* dispatching events regardless of how the relevant option is being changed.

Obviously we should then also dispatch events in `AppOptions.set`, and to avoid adding even more duplicated code this method is changed into a wrapper for `AppOptions.setAll`.
While this is technically a tiny bit less efficient I don't think it matters in practice, since outside of development- and debug-mode `AppOptions.set` is barely used.
  • Loading branch information
Snuffleupagus committed Jul 27, 2024
1 parent 2efa3e4 commit 58c7b5b
Showing 1 changed file with 10 additions and 20 deletions.
30 changes: 10 additions & 20 deletions web/app_options.js
Original file line number Diff line number Diff line change
Expand Up @@ -571,18 +571,7 @@ class AppOptions {
}

static set(name, value) {
const defaultOpt = defaultOptions[name];

if (
!defaultOpt ||
!(
typeof value === typeof defaultOpt.value ||
Type[(typeof value).toUpperCase()] & defaultOpt.type
)
) {
return;
}
userOptions.set(name, value);
this.setAll({ [name]: value });
}

static setAll(options, prefs = false) {
Expand All @@ -601,15 +590,16 @@ class AppOptions {
) {
continue;
}
if (prefs) {
const { kind } = defaultOpt;
const { kind } = defaultOpt;

if (!(kind & OptionKind.BROWSER || kind & OptionKind.PREFERENCE)) {
continue;
}
if (this.eventBus && kind & OptionKind.EVENT_DISPATCH) {
(events ||= new Map()).set(name, userOpt);
}
if (
prefs &&
!(kind & OptionKind.BROWSER || kind & OptionKind.PREFERENCE)
) {
continue;
}
if (this.eventBus && kind & OptionKind.EVENT_DISPATCH) {
(events ||= new Map()).set(name, userOpt);
}
userOptions.set(name, userOpt);
}
Expand Down

0 comments on commit 58c7b5b

Please sign in to comment.