-
Notifications
You must be signed in to change notification settings - Fork 50
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
Feat/spell address hash #200
Conversation
This pull request is being automatically deployed with Vercel (learn more). 🔍 Inspect: https://vercel.com/dux-core-unit/governance-portal-v2/BnPPk5TdL4gRUjjDU2L47wXULtKy |
Codecov Report
@@ Coverage Diff @@
## develop #200 +/- ##
===========================================
- Coverage 38.21% 38.16% -0.05%
===========================================
Files 62 62
Lines 1599 1601 +2
Branches 502 502
===========================================
Hits 611 611
- Misses 981 983 +2
Partials 7 7
Continue to review full report at Codecov.
|
…to feat/spell-address-hash
sx={theme => ({ | ||
background: theme.colors?.background, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This fixes a bug with dark mode. I forgot how much@reach
makes it a pain to use our theme, but this is how it can be done. Supposedly it is compatible with emotion so I have to look into how we can integrate our theme file with reach.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks @b-pmcg , forgot about the dark theme
@@ -34,37 +34,6 @@ const icons = { | |||
</g> | |||
), | |||
viewBox: '0 0 15 15' | |||
}, | |||
hourglass: { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rafinskipg Moved these to the theme so we can use the dai-ui Icon
component, which allows us to pass a color when we set the fill/stroke to currentColor
. Why do we have a clone of the Icon component in the frontend? Is there a namespace issue or something?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LG
TM
use dai-ui icon component and move svg to theme
e8144ae
to
367de96
Compare
Link to Clubhouse story
https://app.shortcut.com/dux-makerdao/story/43/add-spell-details-tab-for-execs
What does this PR do?
Adds spell detail for executive, depends on makerdao/dai.js#296
Steps for testing:
Go to executive detail and see spell info
Screenshots (if relevant):
Any additional helpful information?:
It still needs to add this icon
Add a GIF: