Domains: Update breadcrumbs styling to reflect latest Figma designs #58601

leonardost · 2021-11-29T19:53:45Z

Changes proposed in this Pull Request

When I had implemented #57038, I based the breadcrumbs design on i3 of the domain management pages redesign project (hlVh2q24ad6MCwlwNnE9PQ-fi-600%3A53112), but the latest iteration was i4. This PR updates the styling to reflect the newest designs. Thanks @hambai for noticing this!

The main differences are:

All labels are the same size (13px) and color (--color-neutral-50 = rgb(100, 105, 112) = #646970)
Only the last breadcrumb is bold (font-weight 600) and has color --color-neutral-80 (= rgb(44, 51, 56) = #2c3338)
The separators are lighter in color (--color-neutral-10 = rgb(195, 196, 199) = #c3c4c7)
Breadcrumb links become color --color-neutral-80 when hovered

Check the changes between the following screenshots:

Before:

After:

For the mobile view, the only difference is that the label was darker and now it's lighter.

Before:

After:

Testing instructions

Build this branch locally or open the live Calypso link
Go to "Upgrades > Domains"
Select one of your registered domains or domain connections
If you're in the live Calypso link, please append ?flags=domains/transfers-redesign to your URL to enable the appropriate feature flag
Select the "Transfer your domain" options (or any other page that has breadcrumbs)
Ensure the layout updates described are present

github-actions · 2021-11-29T19:54:05Z

Link to Calypso live: https://calypso.live?image=registry.a8c.com/calypso/app:build-21477
Link to Jetpack Cloud live: https://calypso.live?image=registry.a8c.com/calypso/app:build-21477&env=jetpack

matticbot · 2021-11-29T19:58:56Z

Here is how your PR affects size of JS and CSS bundles shipped to the user's browser:

Sections (~12 bytes added 📈 [gzipped])

name     parsed_size           gzip_size
domains        +48 B  (+0.0%)      +12 B  (+0.0%)

Sections contain code specific for a given set of routes. Is downloaded and parsed only when a particular route is navigated to.

Legend

What is parsed and gzip size?

Parsed Size: Uncompressed size of the JS and CSS files. This much code needs to be parsed and stored in memory.
Gzip Size: Compressed size of the JS and CSS files. This much data needs to be downloaded over network.

Generated by performance advisor bot at iscalypsofastyet.com.

rafaelgallani

LGTM! I tested in both mobile and desktop viewports, and all the changes are reflected. I left a single nitpicky comment, but it should be good to go without addressing it anyway.

rafaelgallani · 2021-11-29T23:26:35Z

client/my-sites/domains/domain-management/components/breadcrumbs/index.jsx

-					className="breadcrumbs__item-label breadcrumbs__item-label--clickable"
-					href={ item.href }
-				>
+				<a className="breadcrumbs__item-label is-clickable" href={ item.href }>


Is there any reason for the is-clickable class? I think it can be removed:

Suggested change

<a className="breadcrumbs__item-label is-clickable" href={ item.href }>

<a className="breadcrumbs__item-label" href={ item.href }>

This should be good to go, as I couldn't find any styles defined for it specifically.
Neither for the breadcrumbs__item-label--clickable class.
I wonder why I added it in the first place 🤔

That's true, it isn't used, good point 🤔 ok, I'll remove it, if we need we can readd it in the future 🙂

rafaelgallani · 2021-11-29T23:32:23Z

@leonardost A special thanks for this one. I'm also to blame for some of the styling divergences, as I added a few of them on #57328 - although I can't remember if that was the latest version at the time, still, thank you 🚀🙌🏻

leonardost · 2021-12-01T14:18:22Z

@rafaelgalani no worries, thank you for the review! 😄

poligilad-auto · 2021-12-01T15:20:38Z

Hey @leonardost, thanks for this.

Some comments:

The main "Domains" page shouldn't have changed:

It should look like it used to, with no hover or link:
For mobile: I guess I didn't add a specific point about this, but the text should actually stay --color-neutral-80.

Another point that I added in later and I mentioned in one of @gius80's PRs is this:
Mobile: header height should be 60px on mobile.
I'm not sure where this belongs.

leonardost · 2021-12-01T20:08:02Z

Thanks for the review @poligilad-auto! I've fixed these regressions - they happened when there was only one breadcrumb item. Let me know if it's ok now:

About the breadcrumb height, @gius80 is doing some modifications to make it sticky in #58519, so we'll fix that there or in a follow up PR 👍

poligilad-auto · 2021-12-03T14:00:11Z

Thanks for the review @poligilad-auto! I've fixed these regressions - they happened when there was only one breadcrumb item. Let me know if it's ok now:

Almost :) On the domains page the title "Domains" should be 16px, like it was before.

For mobile: I guess I didn't add a specific point about this, but the text should actually stay --color-neutral-80.

👍

About the breadcrumb height, @gius80 is doing some modifications to make it sticky in #58519, so we'll fix that there or in a follow up PR 👍

👍

leonardost · 2021-12-06T23:05:53Z

On the domains page the title "Domains" should be 16px, like it was before

Sorry, I had missed that. I updated the font size for the "Domain" and "All domains" breadcrumbs both in desktop and mobile views, now it should be 💯 🙂 I'll deploy this PR, if anything else slipped by we can open another one 👍

…58601) * Update breadcrumbs styling to reflect latest Figma designs * Change last item weight to 500 and add hover color * Remove unused "is-clickable" CSS class from links in breadcrumbs * Fix last breadcrumb item and mobile breadcrumb item stylings * Make lone breadcrumb items have font-weight 600 * Increase font-size of breadcrumb when there's only one item

Update breadcrumbs styling to reflect latest Figma designs

cd9e045

leonardost requested a review from a team as a code owner November 29, 2021 19:53

matticbot added the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Nov 29, 2021

leonardost changed the title ~~Update breadcrumbs styling to reflect latest Figma designs~~ Domains: Update breadcrumbs styling to reflect latest Figma designs Nov 29, 2021

leonardost added the [Feature Group] Emails & Domains Features related to email integrations and domain management. label Nov 29, 2021

rafaelgallani approved these changes Nov 29, 2021

View reviewed changes

leonardost mentioned this pull request Nov 30, 2021

Domains: Fix DNS pages header design issues #58692

Merged

leonardost added 2 commits December 1, 2021 11:05

Change last item weight to 500 and add hover color

6d70cb1

Remove unused "is-clickable" CSS class from links in breadcrumbs

6d5d0c6

leonardost requested a review from a team December 1, 2021 14:28

leonardost added 2 commits December 1, 2021 16:43

Fix last breadcrumb item and mobile breadcrumb item stylings

ddfb0a6

Make lone breadcrumb items have font-weight 600

06b875a

Increase font-size of breadcrumb when there's only one item

2d2417d

leonardost merged commit acbc00c into trunk Dec 6, 2021

leonardost deleted the fix/breadcrumbs-styling branch December 6, 2021 23:19

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 Dec 6, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Domains: Update breadcrumbs styling to reflect latest Figma designs #58601

Domains: Update breadcrumbs styling to reflect latest Figma designs #58601

leonardost commented Nov 29, 2021 •

edited

Loading

github-actions bot commented Nov 29, 2021 •

edited by matticbot

Loading

matticbot commented Nov 29, 2021 •

edited

Loading

rafaelgallani left a comment

rafaelgallani Nov 29, 2021

leonardost Dec 1, 2021

rafaelgallani commented Nov 29, 2021

leonardost commented Dec 1, 2021

poligilad-auto commented Dec 1, 2021

leonardost commented Dec 1, 2021

poligilad-auto commented Dec 3, 2021

leonardost commented Dec 6, 2021

	<a className="breadcrumbs__item-label is-clickable" href={ item.href }>
	<a className="breadcrumbs__item-label" href={ item.href }>

Domains: Update breadcrumbs styling to reflect latest Figma designs #58601

Domains: Update breadcrumbs styling to reflect latest Figma designs #58601

Conversation

leonardost commented Nov 29, 2021 • edited Loading

Changes proposed in this Pull Request

Testing instructions

github-actions bot commented Nov 29, 2021 • edited by matticbot Loading

matticbot commented Nov 29, 2021 • edited Loading

rafaelgallani left a comment

Choose a reason for hiding this comment

rafaelgallani Nov 29, 2021

Choose a reason for hiding this comment

leonardost Dec 1, 2021

Choose a reason for hiding this comment

rafaelgallani commented Nov 29, 2021

leonardost commented Dec 1, 2021

poligilad-auto commented Dec 1, 2021

leonardost commented Dec 1, 2021

poligilad-auto commented Dec 3, 2021

leonardost commented Dec 6, 2021

leonardost commented Nov 29, 2021 •

edited

Loading

github-actions bot commented Nov 29, 2021 •

edited by matticbot

Loading

matticbot commented Nov 29, 2021 •

edited

Loading