[DDW-376] Cleanup SimpleTheme Config in Daedalus #1139
Merged
Conversation
This file contains 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
…operties in ABC order.
|
@MarcusHurney can you please add more info in the PR description? Thanks 🙏 |
…ore/ddw-376-cleanup-simple-theme [DDW-376] Pulls in latest develop branch
…ore/ddw-376-cleanup-simple-theme [DDW-376] Pulls in latests develop branch
|
@nikolaglumac |
|
@MarcusHurney can you please merge in latest |
|
@MarcusHurney i updated to latest |
|
@MarcusHurney @DominikGuzei the changes look good but I have failing tests - it's actually the same problem as with #1156
|
|
@MarcusHurney @DominikGuzei I just got the same issues on develop branch. It could be related to my local setup :-/ |
DominikGuzei
approved these changes
Nov 15, 2018
2 tasks
SebastienGllmt
added a commit
to Emurgo/yoroi-frontend
that referenced
this pull request
Jan 15, 2019
To upgrade to the latest version of `react-polymorph` (v8) we need to do two upgrades: - [x] upgrade to v7.2 (based on [this PR](input-output-hk/daedalus#950)) - [x] upgrade to v8 (based on [this PR](input-output-hk/daedalus#1139)) 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.
Merged
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.
This PR refactors the theme configuration of Daedalus so that the css variables used in
react-polymorphcomponents are distinguishable from other css variables used in Daedalus internally.react-polymorphcss vars are organized into individual objects per component.Prefixes the
react-polymorphcss vars with--rpand renames them to follow a uniform naming convention that describes their use from broad to specific.--rp-formfield-label-text-colorand--rp-formfield-label-text-color-disabled.This convention follows
--rp- prefix+component+element+property+state.Removes the existing configuration of
SimpleThemein Daedalus which importsSimpleThemefrom thereact-polymorphmodule and overrides its scss variables manually.Creates new
react-polymorphcss vars in addition to the existing set. The new ones allow nearly all of the existing css properties within each component to be themed using a configuration object / template. This reduces existing single instance css variables in Daedalus which target properties onreact-polymorphelements that are not currently exposed for customization. Such asfont-size,border,height, andwidth.Creates
source/renderer/app/themes/overridesdirectory for styles that targetreact-polymorphcomponents but aren't configurable through--rpcss vars. These overrides are passed toThemeProvider'sthemeOverridesprop for composition with the base theme.Review Checklist:
Basics
yarn run test)yarn run dev)yarn run package/ CI builds)yarn run flow:test)yarn run lint)yarn run manage:translationsproduces no changes)yarn run storybook)yarn.lockfile is updatedCode Quality
Testing
After Review:
doneon the Youtrack board