Fix MiddlewareApp never properly applying settings passed as props. #836
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
|
|
Bump ^ |
|
🙏 |
idstein
approved these changes
Oct 2, 2018
|
Please, get this in :) |
|
@idstein I can take a look at pulling it out into a module tomorrow if you like |
|
I think we should merge it and release a new version, but currently, I don't have any rights to release so I will let it be here. I think the next release will be there shortly. |
|
Thanks for your first contribution! It will be in next release 🚀Sorry for such a delay |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #826, fixes #796, fixes #791, fixes #790, fixes #748, fixes #796
Basically, the MiddlewareApp component was never actually applying the settings it was passed. I think #759 was an attempted fix, but as PlaygroundWrapper doesn't actually pass settings to the Playground, all this meant was that the ThemeProvider component would take the settings applied over ones from redux. (Not merging them). This caused the weird hidden cursor bug, as if you didn't explicitly set the cursor but did set other options, you lost your cursor. This change means the settings should be merged correctly into the redux state, and the PlaygroundWrapper app can just take the settings from the state as it should.