Skip to content
This repository was archived by the owner on Sep 11, 2024. It is now read-only.

Conversation

t3chguy
Copy link
Member

@t3chguy t3chguy commented Oct 9, 2019

I do not understand why the value would ever come from the SettingsStore directly when we already had a copy of it. If Travis can explain why this is a thing that'd be great.

Fixes issue with preferences not toggling in v1.5.0-rc1

Signed-off-by: Michael Telatynski 7t3chguy@gmail.com

Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>
@t3chguy t3chguy requested a review from a team October 9, 2019 21:30
Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>
@turt2live
Copy link
Member

I do not understand why the value would ever come from the SettingsStore directly when we already had a copy of it. If Travis can explain why this is a thing that'd be great.

The SettingsStore is more reliable than the cached value. Some settings also go through a dialog process, which can revert them. In an effort to make sure the toggle reverts correctly (switches back), it is controlled by the settings store.

@t3chguy
Copy link
Member Author

t3chguy commented Oct 9, 2019

Sure, except here nothing is triggering a re-render so the SettingsStore never gets queried again and thus we end up with stuck toggles

Copy link
Member

@turt2live turt2live left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

otherwise lgtm though. This seems like it'd work, but I'd be more comfortable if we just removed the feature.

@turt2live turt2live added the X-Release-Blocker This affects the current release cycle and must be solved for a release to happen label Oct 9, 2019
Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>
@t3chguy t3chguy requested a review from turt2live October 9, 2019 22:21
@t3chguy t3chguy merged commit 7160922 into develop Oct 10, 2019
@t3chguy t3chguy deleted the t3chguy/fix_150rc1 branch April 27, 2020 18:00
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

X-Release-Blocker This affects the current release cycle and must be solved for a release to happen

2 participants