Skip to content

UI tweaks #12: Final updates! #3787

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.

Sign up for GitHub

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

Jump to bottom
Merged
tvst merged 11 commits into from
Sep 14, 2021
Merged

UI tweaks #12: Final updates! #3787

tvst merged 11 commits into from
Sep 14, 2021

Conversation

@tvst
Copy link
Contributor

@tvst tvst commented Sep 12, 2021

This is the last set of updates! I showed the latest changes to a few people, got their feedback, reverted some decisions and tweaked other aspects.

After this PR, all I need to do is send out a final PR that (1) merges develop and (2) updates all screenshots so I can merge this whole feature/ui-updates branch into develop!

In this PR:

  • Reduce base font to 16px again.
  • Make h1 bolder by adding extrabold font.
  • Make h1 a little bigger.
  • Set vertical rhythm in headers (always add to integer rems). For this I used padding exclusively instead of a mix of padding and margin as before. This allows us to remove the :first-child rules that were used to reduce the top margin when the header is at the top of its .stMarkdown. Those rules only existed because margins are allowed to overlap in some cases but not others (and they didn't even do a great job. See screenshots below). By using padding exclusively I can get rid of those now.
  • Make headers in sidebar smaller (i.e. undo previous change).
  • Add more space to report top.
  • Improve vertical rhythm on st.metric
  • Add typography e2e test. Screenshots coming in next PR.

One way to review this is to go commit-by-commit. They map to the updates above very cleanly.

@tvst tvst requested a review from vdonato September 12, 2021 03:01
@tvst
Copy link
Contributor Author

tvst commented Sep 12, 2021 •
edited
Loading

Here are some screenshots of the new typography.py e2e test for this PR vs the previous state.


General look of our typography

This PR

Screen Shot 2021-09-11 at 8 14 09 PM


Previous state (develop branch)

Screen Shot 2021-09-11 at 8 14 22 PM




Alignment of single st.markdown blocks vs multiple

The left side is one st.markdown block. The right side is a different block for each header.

This PR

Screen Shot 2021-09-11 at 8 15 15 PM


Previous state (develop branch)

Screen Shot 2021-09-11 at 8 14 54 PM


In the image directly above, note how the old headers were all kind of different-looking. The new ones are much more uniform (although that work landed in ui-updates before this PR 😄)

@tvst tvst force-pushed the feature/ui-12-final-updates branch from 0086714 to 39979c9 Compare September 12, 2021 03:37
vdonato
vdonato approved these changes Sep 14, 2021
View reviewed changes
Copy link
Collaborator

@vdonato vdonato left a comment

Choose a reason for hiding this comment

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

A couple small things, but LGTM once those are addressed

Note that we changed our image-taking snapshot recently (upgraded the Cypress version, started using a newer base image, etc), so it'll be necessary to run make build-test-env and go through the struggle of waiting for the long build time to complete before updating snapshots for the next PR

@tvst tvst merged commit 984bdb2 into feature/ui-updates Sep 14, 2021
@tvst tvst deleted the feature/ui-12-final-updates branch September 14, 2021 05:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vdonato vdonato vdonato approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@tvst @vdonato