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

Add tapping functionality to domain cards on the Site Domain screen #19910

Merged
merged 18 commits into from
Jan 12, 2024

Conversation

irfano
Copy link
Member

@irfano irfano commented Jan 9, 2024

This PR makes changes on the Site Domain screen

  • When tapping the cards (except the free domain card), it navigates to the domain detail screen.
domain-detail.webm

To Test:

  1. Navigate to My Site → More → Domains.
  2. Verify the design is as expected.
  3. Tap on a custom domain card.
  4. Verify the domain detail screen is launched.
  5. Test on different orientations and themes.

Regression Notes

  1. Potential unintended areas of impact

    • Nowhere
  2. What I did to test those areas of impact (or what existing automated tests I relied on)

    • Nothing
  3. What automated tests I added (or what prevented me from doing so)

    • Updated DomainsDashboardViewModelTest

PR Submission Checklist:

  • I have completed the Regression Notes.
  • I have considered adding accessibility improvements for my changes.
  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

UI Changes Testing Checklist:

  • Portrait and landscape orientations.
  • Light and dark modes.
  • Fonts: Larger, smaller and bold text.
  • High contrast.
  • Talkback.
  • Languages with large words or with letters/accents not frequently used in English.
  • Right-to-left languages. (Even if translation isn’t complete, formatting should still respect the right-to-left layout)
  • Large and small screen sizes. (Tablet and smaller phones)
  • Multi-tasking: Split screen and Pop-up view. (Android 10 or higher)

@wpmobilebot
Copy link
Contributor

wpmobilebot commented Jan 9, 2024

Jetpack📲 You can test the changes from this Pull Request in Jetpack by scanning the QR code below to install the corresponding build.
App NameJetpack Jetpack
FlavorJalapeno
Build TypeDebug
Versionpr19910-e98bb86
Commite98bb86
Direct Downloadjetpack-prototype-build-pr19910-e98bb86.apk
Note: Google Login is not supported on these builds.

@wpmobilebot
Copy link
Contributor

wpmobilebot commented Jan 9, 2024

WordPress📲 You can test the changes from this Pull Request in WordPress by scanning the QR code below to install the corresponding build.
App NameWordPress WordPress
FlavorJalapeno
Build TypeDebug
Versionpr19910-e98bb86
Commite98bb86
Direct Downloadwordpress-prototype-build-pr19910-e98bb86.apk
Note: Google Login is not supported on these builds.

@ravishanker
Copy link
Contributor

ravishanker commented Jan 10, 2024

Adds > chevron to domain cards (except free domain card)

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, never expires label is not aligned to the right, because of chevron.

@irfano
Copy link
Member Author

irfano commented Jan 10, 2024

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?

irfano and others added 2 commits January 10, 2024 22:21
The logic has been copied from Calypso.
…-label-to-domain-card

Add domain status label to cards on Site Domain screen
@ravishanker
Copy link
Contributor

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?

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

Domain management not opening properly in some cases

@irfano
Copy link
Member Author

irfano commented Jan 11, 2024

@ravishanker, I've removed the chevron and also added a video in the PR description.

Copy link
Contributor

@ravishanker ravishanker left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@ravishanker ravishanker merged commit 54d9f46 into trunk Jan 12, 2024
19 checks passed
@ravishanker ravishanker deleted the feature/update-site-domains-screen branch January 12, 2024 02:52
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