Let settings work with arbitrary input controls
ClosedPublic

Authored by broulik on Sun, Aug 11, 7:10 PM.

Details

Summary

Don't limit it to just check boxes and let the control specify the key to write into.
Check boxes will write their checked state whereas other controls will write their value or a bool for special TRUE or FALSE

Test Plan

This is in preparation for new features that might use a combo box for some settings

Diff Detail

Repository
R856 Plasma Browser Integration
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
broulik created this revision.Sun, Aug 11, 7:10 PM
Restricted Application added a project: Plasma. · View Herald TranscriptSun, Aug 11, 7:10 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
broulik requested review of this revision.Sun, Aug 11, 7:10 PM
fvogt added inline comments.Mon, Aug 12, 5:36 PM
extension/options.js
36–37

Can be removed?

113

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.

116

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

119

console

fvogt requested changes to this revision.Mon, Aug 12, 5:36 PM
This revision now requires changes to proceed.Mon, Aug 12, 5:36 PM
broulik added inline comments.Mon, Aug 12, 8:06 PM
extension/options.js
116

What do you mean?

fvogt added inline comments.Mon, Aug 12, 8:17 PM
extension/options.js
116

<input type="checkbox" data-extension="foo"/> -> <input type="checkbox" data-extension="foo" data-settings-key="enabled"/> and then just error out if there's no key defined.

broulik updated this revision to Diff 63629.Mon, Aug 12, 9:00 PM
broulik edited the summary of this revision. (Show Details)
broulik updated this revision to Diff 63638.Tue, Aug 13, 7:09 AM
  • Don't write config keys for which we don't have a default
fvogt accepted this revision.Tue, Aug 13, 7:27 AM

Didn't test, but looks good

This revision is now accepted and ready to land.Tue, Aug 13, 7:27 AM
This revision was automatically updated to reflect the committed changes.