Skip to content

Quick Start for Existing Users: Move site title waypoint to trailing edge#18651

Merged
momo-ozawa merged 1 commit intorelease/19.9from
task/qs-site-title-waypoint
May 19, 2022
Merged

Quick Start for Existing Users: Move site title waypoint to trailing edge#18651
momo-ozawa merged 1 commit intorelease/19.9from
task/qs-site-title-waypoint

Conversation

@momo-ozawa
Copy link
Copy Markdown
Contributor

@momo-ozawa momo-ozawa commented May 18, 2022

Ref: p5T066-3dZ-p2#comment-12369

not sure if it has changed recently but the Check your site title focus point is being displayed maybe a bit closer to the site image than the to site title?.

Description

Moved the site title waypoint from the leading edge to the trailing edge.

Simulator Screen Shot - iPhone 13 - 2022-05-18 at 15 57 28 Simulator Screen Shot - iPhone 13 - 2022-05-18 at 17 04 35 Simulator Screen Shot - iPhone 13 - 2022-05-18 at 17 07 51

Notes

@osullivanchris I ended up keeping the default offset (i.e. 10 px), since the only time it looks like there's too much padding is when the title is truncated. I tried 2 px, 4 px -- while it looked fine when the title is truncated, I think it's not enough padding for non-trucated situations. Lmk what you think.

2px 2px truncated 4px 4 px truncated
Simulator Screen Shot - iPhone 13 - 2022-05-18 at 17 09 53 Simulator Screen Shot - iPhone 13 - 2022-05-18 at 17 10 36 Simulator Screen Shot - iPhone 13 - 2022-05-18 at 17 11 44 Simulator Screen Shot - iPhone 13 - 2022-05-18 at 17 13 48

How to test

  1. Enable the Quick Start for New Sites via the debug menu or creating a new site
  2. Start the Site Title tour
  3. ✅ Verify: The waypoint is displayed at the trailing edge of the title

Regression Notes

  1. Potential unintended areas of impact
  • RtL languages
  1. What I did to test those areas of impact (or what existing automated tests I relied on)
  • Tested manually
  1. What automated tests I added (or what prevented me from doing so)
  • n/a

PR submission checklist:

  • I have completed the Regression Notes.
  • I have considered adding unit tests for my changes.
  • 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.

@momo-ozawa momo-ozawa added this to the 19.9 ❄️ milestone May 18, 2022
@momo-ozawa momo-ozawa self-assigned this May 18, 2022
@momo-ozawa
Copy link
Copy Markdown
Contributor Author

momo-ozawa commented May 18, 2022

@leandroalonso @hassaanelgarem
FYI I tried using semanticContentAttribute to get the same behavior as the site url spotlight to force the waypoint to be displayed on the trailing edge. This worked for non-truncated titles, but added a weird space in the leading edge for truncated titles... so I opted to explicitly constrain the spotlight's x anchor to the trailing edge.

@wpmobilebot
Copy link
Copy Markdown
Contributor

wpmobilebot commented May 18, 2022

You can test the changes in Jetpack from this Pull Request by:
  • Clicking here or scanning the QR code below to access App Center
  • Then installing the build number pr18651-6616bb0 on your iPhone

If you need access to App Center, please ask a maintainer to add you.

@wpmobilebot
Copy link
Copy Markdown
Contributor

wpmobilebot commented May 18, 2022

You can test the changes in WordPress from this Pull Request by:
  • Clicking here or scanning the QR code below to access App Center
  • Then installing the build number pr18651-6616bb0 on your iPhone

If you need access to App Center, please ask a maintainer to add you.

hassaanelgarem
hassaanelgarem approved these changes May 19, 2022
View reviewed changes
Copy link
Copy Markdown
Contributor

@hassaanelgarem hassaanelgarem left a comment

Choose a reason for hiding this comment

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

:shipit:

@momo-ozawa momo-ozawa merged commit b60366a into release/19.9 May 19, 2022
@momo-ozawa momo-ozawa deleted the task/qs-site-title-waypoint branch May 19, 2022 14:28
@momo-ozawa
Copy link
Copy Markdown
Contributor Author

momo-ozawa commented May 19, 2022

FYI @mokagio 🙇‍♀️

@mokagio mokagio mentioned this pull request May 20, 2022
Merged
7 tasks
@mokagio
Copy link
Copy Markdown
Contributor

mokagio commented May 20, 2022

@momo-ozawa this has been bundled as part of 19.9 beta 2 (19.9.0.2).

Thanks for your work 🙌

@osullivanchris
Copy link
Copy Markdown

osullivanchris commented May 20, 2022

@osullivanchris I ended up keeping the default offset (i.e. 10 px), since the only time it looks like there's too much padding is when the title is truncated. I tried 2 px, 4 px -- while it looked fine when the title is truncated, I think it's not enough padding for non-trucated situations. Lmk what you think.

oh interesting I didn't know there was a default padding @momo-ozawa I can see how it works now.

For me, 4px looks slightly better. But its splitting hairs. If you're finished here, just roll with whatever you have done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@leandroalonso leandroalonso Awaiting requested review from leandroalonso

1 more reviewer

@hassaanelgarem hassaanelgarem hassaanelgarem approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@momo-ozawa momo-ozawa

Labels

None yet

Projects

None yet

Milestone

19.9

Development

Successfully merging this pull request may close these issues.

5 participants

@momo-ozawa @wpmobilebot @mokagio @osullivanchris @hassaanelgarem