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

Upgrade to React-Polymorph v8 #201

Merged
merged 4 commits into from
Jan 15, 2019
Merged

Conversation

SebastienGllmt
Copy link
Contributor

@SebastienGllmt SebastienGllmt commented Dec 28, 2018

To upgrade to the latest version of react-polymorph (v8) we need to do two upgrades:

I made both upgrades as the two commits to this PR and it's probably easier to read if you code review the two commits individually (since the changes are orthogonal). I also have a README change that explains what react-polymorph v8 does but you can find something similar in the Daedalus PR I linked above.

v8 also introduces large refactoring changes that makes a mess of the whole thing. I tried my best to make all the UI look the same while keeping the improvements from v8 but one thing I couldn't stop is that the font size is not 1px bigger across the app (which honestly is not a big deal so I didn't bother). There are probably small things that changed here in there but I manually compared the whole UI and everything looked close enough I don't think anybody would notice any difference that did occur.

@SebastienGllmt SebastienGllmt added the WIP / DO NOT MERGE Shows that a PR shouldn't be merge label Dec 28, 2018
@SebastienGllmt SebastienGllmt force-pushed the feature/update-polymorph branch from 84fa41a to ab9a422 Compare December 29, 2018 12:33
@SebastienGllmt SebastienGllmt added the UI/UX Makes a visual change label Dec 29, 2018
@SebastienGllmt SebastienGllmt changed the title Feature/update polymorph Upgrade to React-Polymorph v8 Dec 29, 2018
@SebastienGllmt
Copy link
Contributor Author

SebastienGllmt commented Dec 29, 2018

Before this is merged we need to

Note: The bug was not fixed in react-polymorph. I just changed our test locally so that it passes. It's not a big deal and I want to get this PR merged to avoid nasty merge conflicts later.

@SebastienGllmt SebastienGllmt force-pushed the feature/update-polymorph branch from adbe55a to ad825a5 Compare January 14, 2019 19:00
@SebastienGllmt SebastienGllmt changed the base branch from feature/update-react to develop January 14, 2019 19:00
@SebastienGllmt SebastienGllmt removed the WIP / DO NOT MERGE Shows that a PR shouldn't be merge label Jan 14, 2019
@NikeOnly NikeOnly self-requested a review January 15, 2019 12:17
@SebastienGllmt SebastienGllmt merged commit 1b93d9f into develop Jan 15, 2019
@SebastienGllmt SebastienGllmt deleted the feature/update-polymorph branch January 15, 2019 13:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
UI/UX Makes a visual change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants