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

Font sizes updated #56

Merged
merged 9 commits into from Jul 10, 2019
Merged

Font sizes updated #56

merged 9 commits into from Jul 10, 2019

Conversation

rkzel
Copy link
Collaborator

@rkzel rkzel commented Jul 2, 2019

This addresses all small updated mentioned in the associated ticket.

Copy link
Collaborator

@chadoh chadoh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would rather we stick with aragonUI standard components as much as possible, even if it results in a slight divergence from our original design. Benefits:

  • when aragonUI gets updated, our interface is automatically updated to match
  • a more standard look/feel across aragon apps

@stellarmagnet what do you think? Most of the changes here cause our Profile UI to diverge from aragonUI standards.

@rkzel
Copy link
Collaborator Author

rkzel commented Jul 10, 2019

Before

56-before

After

56-after

Copy link
Collaborator

@chadoh chadoh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice cleanup! I'd like to see two last small changes before merge, but I don't need to re-review once they're implemented, so I'm going ahead and marking "approved" now.

  1. Rename the new SchoolCompanyName component, as suggested in my comment below
  2. Squash the commits together. I don't think having commits in our history with messages like "experiments" will help us.

src/client/components/styled-components/index.js Outdated Show resolved Hide resolved
@rkzel rkzel merged commit 31fcd2b into master Jul 10, 2019
@rkzel rkzel deleted the 35-font-sizes branch July 10, 2019 20:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants