Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make tab scrolling settings work #553

Merged
merged 4 commits into from Jan 11, 2019

Conversation

Projects
None yet
1 participant
@50Wliu
Copy link
Member

50Wliu 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

50Wliu added some commits Jan 11, 2019

@50Wliu 50Wliu merged commit b39c2b2 into master Jan 11, 2019

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@50Wliu 50Wliu deleted the wl-tab-scrolling branch Jan 11, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.