Skip to content

Conversation

@antonis
Copy link
Contributor

@antonis antonis commented May 6, 2022

Fixes #16393

Description

This PR adds a larger horizontally scrolling section under the title with caption “Picked for you” to prominently present a list of recommended designs.

Note:

To test

Start the site creation flow and verify that the recommended section is shown

Before (feature/site-design-revamp) After
Screenshot_20220504_190123 Screenshot_20220509_115433

Verify that the UI of the page layout picker is unchanged

Before (feature/site-design-revamp) After
2Screenshot_20220504_120843 Screenshot_20220509_115516

Regression Notes

  1. Potential unintended areas of impact
    Page Layout Picker UI

  2. What I did to test those areas of impact (or what existing automated tests I relied on)
    Manual testing and the existing ModalLayoutPickerViewModelTest

  3. What automated tests I added (or what prevented me from doing so)
    Relied/modified on existing tests

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.

@peril-wordpress-mobile
Copy link

peril-wordpress-mobile bot commented May 6, 2022

You can trigger optional UI/connected tests for these changes by visiting CircleCI here.

@antonis antonis added [Type] Task Site Creation [Status] Not Ready for Merge Part of a WIP Feature This label is used to disable milestone checks for PRs that are not against `develop` or `release`. labels May 6, 2022
@antonis antonis changed the base branch from trunk to feature/site-design-revamp May 6, 2022 14:28
@peril-wordpress-mobile
Copy link

peril-wordpress-mobile bot commented May 6, 2022

You can test the changes on this Pull Request by downloading the APKs:

Copy link
Contributor

@ovitrif ovitrif left a comment

Choose a reason for hiding this comment

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

LGTM so far 🎉

Thank you @antonis for the progress you did on this 🙇

Copy link
Contributor

@mkevins mkevins left a comment

Choose a reason for hiding this comment

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

This looks good and is working well for me so far in my testing. Also, the approach is reasonable, imo. I've added a suggestion for how to tackle the recycling issue for the different types of category rows.

Another alternative to consider would be to make the recommended row its own recycler view at the top level (as a preceding sibling outside of the layoutsRecyclerView).

@antonis antonis marked this pull request as ready for review May 9, 2022 08:59
@antonis antonis requested review from mkevins and ovitrif May 9, 2022 08:59
Copy link
Contributor

@ovitrif ovitrif left a comment

Choose a reason for hiding this comment

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

Code changes LGTM 🎉 !

Testing will follow 👍

@ovitrif
Copy link
Contributor

ovitrif commented May 9, 2022

Testing will follow 👍

Works as expected! 🎉

@antonis Should we :shipit: ? 🚀

@antonis
Copy link
Contributor Author

antonis commented May 9, 2022

Thank you for testing and reviewing the code @ovitrif 🙇

@antonis Should we :shipit: ? 🚀

I think we can merge on the feature branch with your approval 🚢

@ovitrif ovitrif merged commit cac586f into feature/site-design-revamp May 9, 2022
@ovitrif ovitrif deleted the task/16393-recommended-designs branch May 9, 2022 13:26
@antonis antonis mentioned this pull request May 24, 2022
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Part of a WIP Feature This label is used to disable milestone checks for PRs that are not against `develop` or `release`. Site Creation [Type] Task

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Site Design Revamp] Main View - Add recommended designs section

4 participants