D23100: Let settings work with arbitrary input controls

Fabian Vogt noreply at phabricator.kde.org
Mon Aug 12 18:36:23 BST 2019


fvogt added inline comments.

INLINE COMMENTS

> options.js:37
>  
>  function extensionCheckboxes() {
>      return document.querySelectorAll("#extensions-selection input[type=checkbox][data-extension]");

Can be removed?

> options.js:114
> +
> +        if (!settings[extension]) {
> +            settings[extension] = {};

Before it would just ignore that, but now it just creates a new settings key if it didn't exist. Any particular reason?

IMO there has to be a default defined for every possible setting anyway.

> options.js:119
> +        let settingsKey = control.dataset.settingsKey;
> +        if (control.type === "checkbox" && !settingsKey) {
> +            settingsKey = "enabled";

AFAIK we control everything on the settings page already, so why not just convert all of them?

> options.js:124
> +        if (!settingsKey) {
> +            consle.warn("Invalid settings key in", control, "cannot save this");
> +            continue;

`console`

REPOSITORY
  R856 Plasma Browser Integration

REVISION DETAIL
  https://phabricator.kde.org/D23100

To: broulik, #plasma, fvogt
Cc: plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20190812/39f92dda/attachment-0001.html>


More information about the Plasma-devel mailing list