-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Domain management revamp: Update single domain page #97240
Conversation
Jetpack Cloud live (direct link)
Automattic for Agencies live (direct link)
|
Here is how your PR affects size of JS and CSS bundles shipped to the user's browser: App Entrypoints (~24 bytes added 📈 [gzipped])
Common code that is always downloaded and parsed every time the app is loaded, no matter which route is used. Sections (~82 bytes added 📈 [gzipped])
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. Generated by performance advisor bot at iscalypsofastyet.com. |
<span>{ itemData.subtitle }</span> | ||
<Icon | ||
className="sidebar-v2__external-icon" | ||
icon={ external } | ||
size={ extraProps?.externalIconSize || ICON_SIZE_SMALL } | ||
/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A small thing I noticed, adding it here while I'm testing the rest.
As this is a common component being used in the Sites dashboard as well, I've noticed a couple of small side-effects of this change on the wordpress.com/sites screen -
- Alignment of the external icon - not aligned with the text anymore
Before | After |
---|---|
![]() |
![]() |
- External icon not at the end of the line in case of line break, also becoming smaller with screen estate instead of having a min-size
Before | After |
---|---|
![]() |
![]() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for catching that! I've reverted the change and it's fixed now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM overall 🚀 Just left a couple of comments
className="sidebar-v2__external-icon" | ||
icon={ external } | ||
size={ extraProps?.externalIconSize || ICON_SIZE_SMALL } | ||
/> | ||
</Button> | ||
) : ( | ||
itemData.subtitle |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Upon opening domain records, the items don't have a line break, and the cards gets wide, but they aren't scrollable. So the content gets cut
My.Movie.mp4
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like this was solved by going full-width on mobile. Could you please test it again?
padding-inline: 0; | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Switching to full-width on mobile managed to solve these issues. Could you please give it another look?
Yap it solves the other issues. But unfortunately, I still see the issue of accordions stretching beyond the screen real estate when opened.
To make it easier to reproduce and fix, I think it happens when one of they values under the DNS records are very long. Here's how you can cause it to happen -
Screen.Recording.2024-12-12.at.1.37.45.AM.mov
b493d0b
to
caab7aa
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the issue is solved! <3 Looks like it just needs a rebase and conflict fix
This Pull Request is now available for translation here: https://translate.wordpress.com/deliverables/17065851 Some locales (Hebrew, Japanese) have been temporarily machine-translated due to translator availability. All other translations are usually ready within a few days. Untranslated and machine-translated strings will be sent for translation next Monday and are expected to be completed by the following Friday. Thank you @m1r0 for including a screenshot in the description! This is really helpful for our translators. |
Related to #96812
Proposed Changes
This PR updates the looks of the single domain overview section following the design - ePfz53j71KKDLWtuSm567A-fi-4948_7391.
There are slight deviations from the design to maintain consistency with the wordpress.com/sites screen. Please keep that in mind when comparing.
Why are these changes being made?
Testing Instructions
Screenshots
Pre-merge Checklist