Skip to content

Conversation

@ovitrif
Copy link
Contributor

@ovitrif ovitrif commented May 17, 2022

Closes #16574

Refactors dependency injection to use Hilt instead of Dagger for activities, fragments & view models of the Site Creation flow.

To test:

  • Run optional UI/connected tests CI step (✅ Done - Green).

  • Run the following test manual testing scenarios with exploratory touches (pressing back, changing screen orientation randomly on the screens).
    If everything works as before then this PR is ok:

1. Site Name feature off (default)

  1. Start the site creation flow
  2. Choose a site intent or skip
  3. Tap a theme to preview it
  4. Tap Choose on the theme preview screen
  5. Search for domain suggestions and choose one
  6. Tap Create Site

2. Site Name feature on

  1. Override SiteNameFeatureConfig.isEnabled() to return true here
  2. Start the site creation flow
  3. Choose a site intent or skip
  4. Enter a site name or skip
  5. Tap a theme
  6. Tap Create Site on the theme preview screen

3. Regression Testing - Page Layout Picker

  1. Create a new site page using the blue FAB button
  2. Tap a layout
  3. Tap Preview
  4. Tap Create Page

Regression Notes

  1. Potential unintended areas of impact

    • Page Layout Picker (Create New Page → Choose a layout)
    • Site creation flow (steps before & after the updated screens)
  2. What I did to test those areas of impact (or what existing automated tests I relied on)

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

    • This PR introduces dependency injection changes — this should not incur changes to automated tests.

Review Notes

  • One reviewer should be enough.
  • I also requested a review from @irfano for a quick check that everything is ok, but we already discussed these changes before so it's not a must.

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 17, 2022

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

@ovitrif ovitrif changed the base branch from trunk to feature/site-design-revamp May 17, 2022 10:48
@ovitrif ovitrif added the Part of a WIP Feature This label is used to disable milestone checks for PRs that are not against `develop` or `release`. label May 17, 2022
@peril-wordpress-mobile
Copy link

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

Copy link
Contributor

@antonis antonis left a comment

Choose a reason for hiding this comment

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

Great work @ovitrif 👍
I've tested on a Pixel 5/Android 12 and everything worked as expected. The code changes also look consistent to me 🎉
Thank you for improving our codebase 🙇

@antonis antonis merged commit 2f24b9f into feature/site-design-revamp May 18, 2022
@antonis antonis deleted the refactor/site-creation-screens-di-with-hilt branch May 18, 2022 07:30
@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] Enhancement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Site Creation] Replace Dagger with Hilt for Dependency Injection

3 participants