Skip to content
This repository was archived by the owner on Dec 15, 2022. It is now read-only.

Conversation

@winstliu
Copy link
Contributor

@winstliu winstliu commented Jan 11, 2019

Requirements

  • Filling out the template is required. Any pull request that does not include enough information to be reviewed in a timely manner may be closed at the maintainers' discretion.
  • All new code requires tests to ensure against regressions

Description of the Change

Where to even start? The tabScrolling setting uses a mixed enum of booleans and strings, and declares its type as any. This should be fine...except that since settings-view determines the value of a setting by looking at the HTML value, all booleans are coerced to strings. Then atom.config.set decides that the new stringified value is not valid as it isn't in the valid list of values for tabScrolling and ignores it. The "fix" is to explicitly declare the enum as having types boolean and string, so that atom.config.set will coerce the stringified boolean back to an actual boolean before checking it against the schema...

Oh, and I also fixed being unable to disable tab scrolling after you enabled it without restarting the tabs package, as the event listener wasn't being removed (function equality). The event listener is now always active but only does anything if the tabScrolling setting is enabled.

Alternate Designs

Fix settings-view, because there's nothing technically wrong with how tabs decided to implement this setting.

Benefits

Changing the tab scrolling setting through settings-view will work again, and tab scrolling can be disabled on the fly.

Possible Drawbacks

None.

Applicable Issues

Fixes #311

@winstliu winstliu merged commit b39c2b2 into master Jan 11, 2019
@winstliu winstliu deleted the wl-tab-scrolling branch January 11, 2019 23:07
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.

Tab scrolling stopped working

2 participants