-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Add tapping functionality to domain cards on the Site Domain screen #19910
Conversation
IDE suggestion
The result of the `all-domains` endpoint will be used for implementing new features.
📲 You can test the changes from this Pull Request in Jetpack by scanning the QR code below to install the corresponding build.
|
📲 You can test the changes from this Pull Request in WordPress by scanning the QR code below to install the corresponding build.
|
Chevrons are an iOS pattern. Android doesn't use chevrons on cards and list items. Probably no one noticed it when they were doing the Domain Management screens. Designers often design for iOS first, and copy them over! We had discussion on this many times. For e.g. Stats on iOS has chevrons on cards, and Android doesn't. Chevrons also introduce other side effects. On the other PR #19912 , see how first card in after image, |
Good point, @ravishanker! What is your suggestion? If I remove the chevron, it might look like as a bug when the user can't tap the free domain card. Do you think it is fine to simply remove the chevron, or should we also add an icon to the card, like this? |
The logic has been copied from Calypso.
…-label-to-domain-card Add domain status label to cards on Site Domain screen
Just remove the chevron. No need for another icon there. Make the whole card clickable. That would also fix the issue @staskus mentioned on the other PR
|
@ravishanker, I've removed the chevron and also added a video in the PR description. |
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 👍
This PR makes changes on the Site Domain screen
domain-detail.webm
To Test:
Regression Notes
Potential unintended areas of impact
What I did to test those areas of impact (or what existing automated tests I relied on)
What automated tests I added (or what prevented me from doing so)
DomainsDashboardViewModelTest
PR Submission Checklist:
RELEASE-NOTES.txt
if necessary.UI Changes Testing Checklist: