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

Domains: Redesigned domain list updates #58980

Merged
merged 5 commits into from Dec 10, 2021

Conversation

leonardost
Copy link
Contributor

@leonardost leonardost commented Dec 9, 2021

Changes proposed in this Pull Request

This PR addresses the design review issues pointed out in pcYYhz-pX-p2#comment-388, pcYYhz-pX-p2#comment-389 and pcYYhz-pX-p2#comment-390, which were:

  1. "Get your domain" card appearing above the domain filters dropdown

Markup on 2021-12-08 at 20:12:27

  1. When clicking the "All my domains" option in the domain filters dropdown, the user was sent to the "All domains" page with the "All domains" filter selected instead of the "Owned by me" filter
  2. Switch the position of the "+" sign in the "Create site +" and "Add +" (email) links in the domains list

Markup on 2021-12-08 at 20:03:51

Markup on 2021-12-08 at 20:09:32

You can compare them with the previous designs:

Screen Shot 2021-12-08 at 20 36 00

Markup on 2021-12-08 at 20:38:06

Markup on 2021-12-08 at 20:39:17

Testing instructions

  • Build this branch locally or open the live Calypso link
    • If you're in the live Calypso link, please append ?flags=domains/management-list-redesign to your URL to enable the appropriate feature flag

Points to verify:

  1. "Get your domain" card position
  • Select a site with a plan where you haven't used domain credits yet, i.e. you didn't claim your free domain
  • Go to "Upgrades > Domains"
  • Check if the "Get your domain" card appears in the correct location in mobile view
  1. Clicking "All my domains" filter option
  • In the domains filter dropdown, select "All my domains" and ensure you're taken to the "All domains" page with the "Owned by me" filter selected
  1. "+ Create site" and "+ Add" links
  • In the "All domains" page, ensure that the "Create site +" link reads "+ Create site" now - this link appears for domain-only sites
  • Select a site where you have domains without email configured, go to "Upgrades > Domains" and ensure the "Add +" link in the "Email" column reads "+ Add" now

@leonardost leonardost requested a review from a team as a code owner December 9, 2021 00:06
@matticbot matticbot added the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Dec 9, 2021
@github-actions
Copy link

github-actions bot commented Dec 9, 2021

@matticbot
Copy link
Contributor

matticbot commented Dec 9, 2021

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

Sections (~19 bytes added 📈 [gzipped])

name     parsed_size           gzip_size
domains        +77 B  (+0.0%)      +19 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.

@@ -284,7 +284,7 @@ class DomainRow extends PureComponent {

return (
<a href="#" onClick={ this.addEmailClick }>
{ translate( 'Add +', { context: 'Button label' } ) }
{ translate( '+ Add', { context: 'Button label' } ) }
Copy link

Choose a reason for hiding this comment

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

🆗 This change will be queued for retranslation. We'll use the existing translations in the meantime.

@leonardost leonardost added the [Feature Group] Emails & Domains Features related to email integrations and domain management. label Dec 9, 2021
@leonardost leonardost requested a review from a team December 9, 2021 00:45
@poligilad-auto
Copy link

Nice work @leonardost and thanks.
All is working as expected.

There is just a strange functionality that happened to me:
I started with a site with no domains > selected "All my domains" > saw "All domains" but the list was empty > I switched to a site where I own 2 domains > selected "All my domains" > saw "All domains" and now I saw the list with 2 domains that I own (but I own more) > switched to another site with a domain > selected "All my domains" > saw "All domains" with 3 domains that I own 🤷‍♀️

Hope my explanation is clear :)

@poligilad-auto
Copy link

Also @leonardost, really sorry for not noticing this before.
In the designs the big version (when there are also no connected domains) of the banner doesn't have any borders (as we have enough on the page as is).
Could we also make that update here?

image

@leonardost
Copy link
Contributor Author

Thanks for the review @poligilad-auto!

Could we also make that update here?

No worries, sure thing! Should the border be removed in mobile view too, like this?

Screen Shot 2021-12-09 at 12 24 55

I started with a site with no domains > selected "All my domains" > saw "All domains" but the list was empty > I switched to a site where I own 2 domains > selected "All my domains" > saw "All domains" and now I saw the list with 2 domains that I own (but I own more) > switched to another site with a domain > selected "All my domains" > saw "All domains" with 3 domains that I own 🤷‍♀️

This is weird 🤔 I can't seem to reproduce this in my account, every time I select "All my domains" the same domains appear in my list.

Each time you clicked on "All my domains" were you taken to the "All domains" page with the "Owned by me" filter selected? Does this behavior (different domains appearing in the list) occur every time if you follow the same steps?

@gius80 gius80 self-requested a review December 10, 2021 08:18
@gius80
Copy link
Contributor

gius80 commented Dec 10, 2021

This is weird 🤔 I can't seem to reproduce this in my account, every time I select "All my domains" the same domains appear in my list.

I also can not reproduce it.🤯

I still see the border on the banner but I guess you're waiting for the feedback for mobile version, but everything else looks ok to me! 🙂

@leonardost
Copy link
Contributor Author

I'll be deploying this PR so we can start preparing to disable the domains list feature flag and launch for customers 🙂 if anything comes up we can address it in a following PR

@leonardost leonardost merged commit 7e51f28 into trunk Dec 10, 2021
@leonardost leonardost deleted the update/redesigned-domain-list-updates branch December 10, 2021 14:51
@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 Dec 10, 2021
@a8ci18n
Copy link

a8ci18n commented Dec 10, 2021

This Pull Request is now available for translation here: https://translate.wordpress.com/deliverables/7032359

Thank you @leonardost for including a screenshot in the description! This is really helpful for our translators.

@poligilad-auto
Copy link

@leonardost

No worries, sure thing! Should the border be removed in mobile view too, like this?

Thanks! And yes, exactly.

This is weird 🤔 I can't seem to reproduce this in my account, every time I select "All my domains" the same domains appear in my list.

Each time you clicked on "All my domains" were you taken to the "All domains" page with the "Owned by me" filter selected? Does this behavior (different domains appearing in the list) occur every time if you follow the same steps?

That's strange.. Yes, it happens every time. It's as if the current site domains are "added" to the "all domains" list when I visit that site. Check it out:

Strange.mov

@leonardost
Copy link
Contributor Author

@poligilad-auto thanks for the recording! And that's so odd 🤔 I've created a card in Asana for us to keep track and investigate that issue further 1146722293412575-as-1201515666298451

@a8ci18n
Copy link

a8ci18n commented Dec 17, 2021

Translation for this Pull Request has now been finished.

leonardost added a commit that referenced this pull request Dec 23, 2021
In the all domains page we do a `GET /all-domains/` query, which
retrieves all the domains for all the sites of a user. So at least the
initial domain list render should be quick, because the `all-domans`
query is pretty fast even when you have many domains.

The weird behavior reported in #58980 (comment)
was happening because the `createLightSiteDomainObject` method was not
processing the `current_user_is_owner` property - it was only set for
domains that were fully loaded before.

This PR adds this property and removes the `AllSitesDomains` query that
was being done upfront and increased the page's load time.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature Group] Emails & Domains Features related to email integrations and domain management.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants