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

My Site: fix iPad default selected row #18118

Merged
merged 8 commits into from
Mar 15, 2022

Conversation

leandroalonso
Copy link
Contributor

@leandroalonso leandroalonso commented Mar 10, 2022

Part of #17873
Fixes #17958

This PR fixes a bug that has been there for quite some time: the default selected row when using iPad.

To test

With My Site Dashboard flag disabled:

  1. Do a clean install of the app on an iPad
  2. Login with an account with multiple sites
  3. Accept the help when prompted
  4. ✅ "Stats" should be the selected row
  5. Tap any other section
  6. Switch to another site
  7. ✅ "Stats" should be the selected row

With My Site Dashboard flag enabled

  1. Enable My Site Dashboard feature flag
  2. Close and reopen the app
  3. ✅ "Stats" should be the selected row
  4. Tap "Home"
  5. Switch to another site
  6. ✅ "Stats" should be the selected row

Make sure to check the site that has Quick Start enabled too, "Stats" should be the selected row there.

My Site Dashboard flag enabled and dashboard as the default

  1. Create a new site (this will make the Dashboard the default)
  2. ✅ "Home" should be the selected row
  3. Switch to another site
  4. ✅ "Home" should be the selected row

Regression Notes

  1. Potential unintended areas of impact
    iPhone

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

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

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.

@wpmobilebot
Copy link
Contributor

wpmobilebot commented Mar 10, 2022

You can test the Jetpack changes on this Pull Request by downloading it from AppCenter here with build number: pr18118-f1af20c. IPA is available here. If you need access to this, you can ask a maintainer to add you.

@wpmobilebot
Copy link
Contributor

wpmobilebot commented Mar 10, 2022

You can test the WordPress changes on this Pull Request by downloading it from AppCenter here with build number: pr18118-f1af20c. IPA is available here. If you need access to this, you can ask a maintainer to add you.

Copy link
Contributor

@pachlava pachlava left a comment

Choose a reason for hiding this comment

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

From my side, it all looks good! Tested on iPad simulator, and also checked for regressions on iPhone. Thanks, @leandroalonso!

Base automatically changed from issue/17873-fix-site-creation-flow to trunk March 11, 2022 12:46
Copy link
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.

Works as described except for one issue.

After switching from a site that has Quick Start enabled to a site that doesn't have it enabled. The shown section is "Stats" but the selected cell is "Posts"

Copy link
Contributor

@momo-ozawa momo-ozawa left a comment

Choose a reason for hiding this comment

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

I'm seeing a something similar to what @hassaanelgarem mentioned above -- when switching from a site that has Quick Start enabled to a P2 site, "Stats" is shown but "Menus" is selected

@leandroalonso
Copy link
Contributor Author

@momo-ozawa @hassaanelgarem the issues you were seeing should be fixed with the last commits. Could you please try again?

Context: we have 16 BlogDetailsSubsection and @unknown requires the switch to be exhaustive. However, that was never the case. I removed it to suppress the warning.
Copy link
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.

Yes, the issue is fixed for me 🚀

Copy link
Contributor

@momo-ozawa momo-ozawa left a comment

Choose a reason for hiding this comment

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

LGTM!

@leandroalonso leandroalonso merged commit bcf72b9 into trunk Mar 15, 2022
@leandroalonso leandroalonso deleted the issue/17873-fix-ipad-selection branch March 15, 2022 13:20
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.

My Site: Incorrect menu item highlighted after switching site on iPad
5 participants