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

Top bar: Spacing tweaks to match front end #51720

Merged
merged 9 commits into from
Apr 9, 2021
Merged

Conversation

sixhours
Copy link
Contributor

@sixhours sixhours commented Apr 6, 2021

Changes proposed in this Pull Request

  • Lots of minor spacing tweaks to the top bar for closer visual alignment with the front end
  • Rearranged the top bar portions of the stylesheet because I couldn't keep track of where I was otherwise. :P

You'll notice that in some cases the top bar is still not a 1:1 replica between Calypso and the front end; I didn't want to go down the rabbit hole of pixel-perfection, this should iron out the most obvious kinks. If we want 100% visual parity I can continue picking away at pixels. :)

Visual differences by screen size

The front end toolbar has not changed in this PR; I only include the images for comparison

  • Small - few differences before and after

Before

Screen Shot 2021-04-07 at 12 17 07 PM

After

Screen Shot 2021-04-07 at 12 17 01 PM

Front end

Screen Shot 2021-04-07 at 12 16 54 PM

  • Medium - most notable differences before and after, particularly around padding/spacing

Before
Screen Shot 2021-04-07 at 11 11 46 AM

After
Screen Shot 2021-04-07 at 11 11 57 AM

Front end
Screen Shot 2021-04-07 at 11 12 05 AM

  • Large - should be barely any differences before and after

Before

Screen Shot 2021-04-07 at 12 18 09 PM

After

Screen Shot 2021-04-07 at 12 18 29 PM

Front end

Screen Shot 2021-04-07 at 12 25 23 PM

Testing instructions

  • Switch to this PR
  • Check out the top bar in Calypso on multiple pages, multiple devices, multiple screen sizes. Any edge cases I've missed? Anything that looks visibly broken?

Related to #49154

@sixhours sixhours requested a review from a team April 6, 2021 19:36
@sixhours sixhours self-assigned this Apr 6, 2021
@matticbot
Copy link
Contributor

@matticbot matticbot added [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Apr 6, 2021
@matticbot
Copy link
Contributor

This PR does not affect the size of JS and CSS bundles shipped to the user's browser.

Generated by performance advisor bot at iscalypsofastyet.com.

@sixhours sixhours changed the title (WIP) Nav Unification: Spacing tweaks for top bar to match front end Top bar: Spacing tweaks to match front end Apr 7, 2021
Copy link
Contributor

@mreishus mreishus left a comment

Choose a reason for hiding this comment

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

This is working well for me. I only noticed one minor issue which isn't a show stopper. At some widths (mine was 878px), the rightmost line in the new post icon is cut off. This might have to do with me using Firefox or having a double digit number of drafts.

2021-04-08_17-36

@sixhours sixhours merged commit 43ca81a into trunk Apr 9, 2021
@sixhours sixhours deleted the fix/top-bar-spacing branch April 9, 2021 14:37
@github-actions github-actions bot removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Apr 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants