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

Fix dark mode slider switch not disabling dark mode on first try #3754

Closed
wants to merge 1 commit into from

Conversation

wiz
Copy link
Contributor

@wiz wiz commented Dec 6, 2019

Apparently the preferences property listener I was using doesn't call
the event handler on the first change for some reason. This is a
workaround to change the CSS theme directly from the slider switch.

Needs review and testing! Fixes #3447

Apparently the preferences property listener I was using doesn't call
the event handler on the first change for some reason. This is a
workaround to change the CSS theme directly from the slider switch.

Needs review and testing! Fixes bisq-network#3447
@wiz wiz requested review from ripcurlx and sqrrm as code owners December 6, 2019 04:10
@ripcurlx ripcurlx added this to the v1.2.5 milestone Dec 6, 2019
@ripcurlx
Copy link
Contributor

ripcurlx commented Dec 6, 2019

I'll have a look at this behavior today and if it caused issues in other parts of the code as well.

@ripcurlx ripcurlx self-assigned this Dec 6, 2019
@ripcurlx
Copy link
Contributor

ripcurlx commented Dec 9, 2019

@wiz The problem is that private final IntegerProperty cssThemeProperty = new SimpleIntegerProperty(prefPayload.getCssTheme()); gets initialized before the stored payload is initialized which causes the value of the cssThemeProperty always to set to 0. That is also the reason why the listener is not triggered the first time when changing back to light mode after a restart.

@ripcurlx
Copy link
Contributor

ripcurlx commented Dec 9, 2019

You missed to update the value in readPersisted in the Preferences class (L:271+). But I also saw during investigation that we are still having properties there we don't use anymore so I'll create another PR cleaning up the unused properties and fixing this issue as well.

@ripcurlx ripcurlx removed this from the v1.2.5 milestone Dec 9, 2019
@ripcurlx
Copy link
Contributor

ripcurlx commented Dec 9, 2019

Closing as superseded by #3771 .

@ripcurlx ripcurlx closed this Dec 9, 2019
@wiz wiz deleted the fix-dark-mode-slider-switch branch December 10, 2019 11:00
@wiz
Copy link
Contributor Author

wiz commented Dec 10, 2019

The problem is that private final IntegerProperty cssThemeProperty = new SimpleIntegerProperty(prefPayload.getCssTheme()); gets initialized before the stored payload is initialized which causes the value of the cssThemeProperty always to set to 0

Ah that's why - good thing it was my bug and not something weird elsewhere :)
Thanks for fixing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Dark mode not disabled on 1st attempt to turn it off after restart
2 participants