-
Notifications
You must be signed in to change notification settings - Fork 212
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
Agent: update help text #1169
Agent: update help text #1169
Conversation
Voting: update VoteText's address rendering (aragon#1146)
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.
Hey, thanks a lot for doing this! A comment:
Let's remove them with this change as well :) |
- Also removes FrameSvg.js
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.
Another comment before we can get this in!
width: 46px; | ||
height: 46px; |
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.
As a rule we avoid setting sizes with numbers, and always use the GU
import with corresponds to 8 pixels, like this: width: ${6 * GU}px
. This way we're more consistent. Let's GUify this!
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.
Ah, I see. Done!
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 should be good to go now! 🔥
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.
LGTM!
Fixes #1162
@sohkai ended up leaving the InstallFrame.js component (and modal) in for now, but will delete if you don't think either are necessary to keep around.