Jetpack focus: Show full screen overlay for Jetpack Features by AjeshRPai · Pull Request #17432 · wordpress-mobile/WordPress-Android

AjeshRPai · 2022-11-07T09:23:12Z

This PR adds a feature-specific overlay for the Feature Removal Phase one. Screenshot of the feature-specific overlays is shown below in dark as well light mode.

Stats	Notifications	Reader

Known issues and Improvements

The UI in landscape mode will be taken up in a separate PR
Analytics and tracking will be taken up in a separate PR
The logic for showing overlay in Site creation will be taken up in a separate PR

Testing Instructions

1. Make sure that the Overlay is not shown

If Phase one is not started yet

Launch the WordPress app with a WP site and navigate to Stats/Reader/Notifications
Verify that the feature overlay is not shown

If the site is a Self-hosted site
No shown on the Jetpack app

2. Jetpack overlay is shown when Phase one is started

The logic for testing whether the overlay is shown according to feature-specific overlay frequency or global-specific overlay frequency is implemented in JetpackFeatureRemovalOverlayUtilTest.

Launch WordPress app and go to My Site -> App Settings -> Debug settings -> enable jp_removal_one
Close app and Launch the app again

Stats

Navigate to Stats either through dashboard/widget/notifications
Verify that the feature-specific overlay is shown
Turn on dark mode
Verify that the overlay is shown in Dark mode
Click on Try the new jetpack app button
Verify that the user is redirected to the Jetpack app on the play store
Navigate to Stats either through dashboard/widget/notifications
Verify that the feature-specific overlay is shown
Click on Continue to Stats button
Notice that the overlay is dismissed and Stats are visible

Notifications feature

Navigate to the Notifications tab either through the dashboard/system notifications
Verify that the feature-specific overlay is shown
Turn on dark mode
Verify that the overlay is shown in Dark mode
Click on Try the new jetpack app button
Verify that the user is redirected to the Jetpack app on the play store
Navigate to the Notifications tab either through the dashboard/notifications
Verify that the feature-specific overlay is shown
Click on Continue to Notifications button
Notice that the overlay is dismissed and Notifications are visible

Reader

Navigate to Reader either through the dashboard/notifications
Verify that the feature-specific overlay is shown
Turn on dark mode
Verify that the overlay is shown in Dark mode
Click on Try the new jetpack app button
Verify that the user is redirected to the Jetpack app on the play store
Navigate to Reader either through the dashboard/notifications
Verify that the feature-specific overlay is shown
Click on Continue to Reader button
Notice that the overlay is dismissed and Notifications are visible

3. Make sure that the Unit tests logic is correct

Unit tests in JetpackFeatureRemovalOverlayUtilTest,
Unit tests in ReaderViewModelTest.kt
Unit tests in StatsViewModelTest

Regression Notes

Potential unintended areas of impact
Overlay is not shown properly
What I did to test those areas of impact (or what existing automated tests I relied on)
Manual testing and Automated tests
What automated tests I added (or what prevented me from doing so)
Added unit tests in JetpackFeatureRemovalOverlayUtilTest, ReaderViewModelTest and StatsViewModelTest

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.

…helper' into issue/17330-jetpack-focus-create-generic-fullscreen-overlay

Adds: new utility class for handling the feature overlay logic Adds: logic to the JetpackFeatureOverlayShownTracker for handling different phases

Removes redundant functions

wpmobilebot · 2022-11-07T09:36:39Z

📲 You can test these changes on Jetpack by downloading jetpack-installable-build-pr17432-eb837a0.apk

	💡 Scan this QR code with your Android phone to download and install the APK directly on it.
	App	`Jetpack`
	Build Flavor	`Jalapeno`
	Build Type	`Debug`
	Commit	`eb837a0`

Note: This installable build uses the JalapenoDebug build flavor, and does not support Google Login.

wpmobilebot · 2022-11-07T09:38:04Z

📲 You can test these changes on WordPress by downloading wordpress-installable-build-pr17432-eb837a0.apk

	💡 Scan this QR code with your Android phone to download and install the APK directly on it.
	App	`WordPress`
	Build Flavor	`Jalapeno`
	Build Type	`Debug`
	Commit	`eb837a0`

Note: This installable build uses the JalapenoDebug build flavor, and does not support Google Login.

1.Updates the Jetpack overlay xml name 2.Adds the click logic to jetpack overlay

1. Updates Jetpack connected feature enum class 2. Adds Overlay screen type to keep track of which screen to be shown 3. Updates the References of Screen type in viewmodel and tests 4. Adds logic for storing the timestamp in which the feature was shown

…llscreen-overlay # Conflicts: # WordPress/src/main/res/values/strings.xml

zwarm

👋 @AjeshRPai - I took a first pass at the code and have a few small comments. I need to dig in deeper into the "shown" logic and dates. I'll give that a go during my next review, but wanted to give you enough to keep moving.

I'm impressed with the work so far. 👏 ❤️

See below for preliminary comments

Should .idea/checkstyle-idea.xml be checked in or should this be removed from the PR?
Nit: Run a reformat code on the android:id="@+id/caption" TextView in jetpack_feature_removal_overlay
In JetpackFeatureFullScreenOverlayFragment how do you feel about replacing
private val binding get() = _binding!!.
with
private val binding get() = _binding ?: throw NullPointerException("_binding cannot be null")
to eliminate the double bang (!!)
Also in JetpackFeatureFullScreenOverlayFragment, wdyt about making the rtlLayout fun name a little bit clearer. The fun is not only checking the rtl is true/false, but also is setting the layoutDirection of the view. BTW: There is a RtlUtils that has a static method called isRtl. Hard to test without a wrapper and may not be worth it to change, but just thought I'd mention it.
In JetpackBrandingUtils - I don't think you added any changes other than the "," at the end of the private val analyticsTrackerWrapper: AnalyticsTrackerWrapper,. Did you intend to add something here? If not, want to revert this file.

Removes: Bottom sheet handle UI from the removal overlay fragment Adds: Close button to the top of the view

peril-wordpress-mobile · 2022-11-17T16:25:52Z

	Warnings
⚠️	PR has more than 300 lines of code changing. Consider splitting into smaller PRs if possible.

Generated by 🚫 dangerJS

AjeshRPai · 2022-11-17T17:45:36Z

@JB184351 @shaunandrews @zwarm 👋🏼

👀 This PR is ready for a re-review now. I have

Removed the swipe to dismiss logic
Added the dismiss button
Removed the testing logic

@JB184351
In order to test the overlay frequency logic you can use the Draft PR - #17475

In addition to the PR, the tests for the frequency logic exist in the class JetpackFeatureRemovalOverlayUtilTest

cc:@zwarm

shaunandrews · 2022-11-18T19:15:11Z

With the "X" close button added, I noticed that it doesn't line up with the rest of the screen:

I also looked a little closer at the colors for the button, and it seems they're using a lighter green which produces a contrast issue:

In the designs we're using jetpack-green-50 (#8710) for the greens.

enejb · 2022-11-18T19:20:33Z

@AjeshRPai jetpack-green-50 has a hex value of #008710

AjeshRPai · 2022-11-21T09:38:47Z

@shaunandrews Thanks for reviewing the PR.

@enejb Thanks for providing the jetpack-green-50 values.

I have updated the UI with

Aligning the close button with the rest of the screen

Updated the colour of the Primary button and the text in the secondary button jetpack-green-50

Secondary button text colour	Primary button colour background

Commit - 1ce8fe5

In case anyone is wondering how I checked the alignment and colour of the elements in the UI - I am using this app on my physical device to check the alignment and colour of the elements as shown on the screenshot.

zwarm

@AjeshRPai - with the exception of landscape mode, all is working as expected.
I ran all the unit tests + manual testing on a device (WP & JP)

Thanks for all the great work 👍

osullivanchris · 2022-11-21T16:28:43Z

Alignment and color looks good in the updated screenshots, thanks @AjeshRPai 👍

…llscreen-overlay # Conflicts: # WordPress/src/main/res/values/strings.xml

AjeshRPai added 15 commits October 27, 2022 16:39

Adds: JetpackOverlayUIState for overlay content

3d0101f

Adds:Overlay content builder for Jetpack Featured Overlay

34d90d4

Merge branch 'issue/17332-jetpack-focus-create-jetpack-current-phase-…

ce55735

…helper' into issue/17330-jetpack-focus-create-generic-fullscreen-overlay

Updates: the reference to sealed class representing the current phase

cbbbb56

Adds: Viewmodel for jetpack full screen overlay

b42c449

Adds: Fragment and UI xml for overlay

bccb4ca

Updates: Naming of Fragment and viewmodel for full screen overlay

d4b0cf1

Adds: Class for tracking the jetpack features accessed

3dedeec

Adds: Functions for determining whether the overlay is shown or not

53882fe

Updates: Utility and Tracking logic for feature overlays

3661f2e

Adds: new utility class for handling the feature overlay logic Adds: logic to the JetpackFeatureOverlayShownTracker for handling different phases

Updates: Util class

a3963ea

Removes redundant functions

Updates: Overlay fragment logic to fix play animation

f3806ff

Adds: jetpack overlay to stats fragment and viewmodel

85264a2

Adds: jetpack overlay to reader viewmodel and fragment

4b6771d

Adds: Jetpack overlay logic to notification fragment and viewmodel

e014b4c

AjeshRPai added the Jetpack label Nov 7, 2022

AjeshRPai added this to the 21.2 milestone Nov 7, 2022

AjeshRPai self-assigned this Nov 7, 2022

AjeshRPai added 9 commits November 7, 2022 15:30

Updates: xml variable for strings

9c21797

Updates: Jetpack overlay logic

38b7a6a

1.Updates the Jetpack overlay xml name 2.Adds the click logic to jetpack overlay

Fixes: crash on phase two enabled scenario

7b24798

Fixes: Lint issues

5c0ee9b

Updates: the primary button text

1f9749d

Merge branch 'trunk' into issue/17330-jetpack-focus-create-generic-fu…

75f1123

…llscreen-overlay # Conflicts: # WordPress/src/main/res/values/strings.xml

Reverts: unintended changes to checkstyle-idea

8ef6063

Updates Checkstyle-idea.xml

ebc2fc8

zwarm reviewed Nov 8, 2022

View reviewed changes

AjeshRPai added 4 commits November 17, 2022 13:33

Removes: Swpie to dismiss behaviour of overlay

cbb8bc5

Updates: Jetpack full screen overlay fragment

e594259

Removes: Bottom sheet handle UI from the removal overlay fragment Adds: Close button to the top of the view

Updates: function naming

e890ff8

Reverts:commented out code for testing purposes

4139dd3

AjeshRPai removed the [Status] Not Ready for Review label Nov 17, 2022

AjeshRPai added 3 commits November 17, 2022 19:49

Updates: Whenever import

3fa1518

Removes: testing logic

e39622e

Fixes: test failure

f162b19

AjeshRPai added 1 and removed 1 labels Nov 17, 2022

Fixes: lint failures

89d8483

AjeshRPai mentioned this pull request Nov 17, 2022

Do not merge - Only here to trigger build for testing Overlay frequency logic in the PR #17475

Closed

3 tasks

AjeshRPai requested a review from zwarm November 18, 2022 07:04

AjeshRPai mentioned this pull request Nov 21, 2022

Jetpack Focus: Display feature specific overlays #17333

Closed

3 tasks

Updates: Ui according to the design review comments

1ce8fe5

zwarm approved these changes Nov 21, 2022

View reviewed changes

ovitrif mentioned this pull request Nov 22, 2022

Make 'WordPress is better with Jetpack' bottom sheet text and button fully visible with big fonts #17503

Merged

3 tasks

JB184351 approved these changes Nov 22, 2022

View reviewed changes

Merge branch 'trunk' into issue/17330-jetpack-focus-create-generic-fu…

eb837a0

…llscreen-overlay # Conflicts: # WordPress/src/main/res/values/strings.xml

AjeshRPai enabled auto-merge November 22, 2022 18:01

AjeshRPai merged commit 5434beb into trunk Nov 22, 2022

AjeshRPai deleted the issue/17330-jetpack-focus-create-generic-fullscreen-overlay branch November 22, 2022 18:17

This was referenced Nov 23, 2022

Jetpack focus display Site creation overlay #17493

Merged

Fixes: Try the jetpack app not being shown properly when the font size is large #17540

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Jetpack focus: Show full screen overlay for Jetpack Features#17432

Jetpack focus: Show full screen overlay for Jetpack Features#17432
AjeshRPai merged 45 commits intotrunkfrom
issue/17330-jetpack-focus-create-generic-fullscreen-overlay

AjeshRPai commented Nov 7, 2022 •

edited

Loading

Uh oh!

wpmobilebot commented Nov 7, 2022 •

edited

Loading

Uh oh!

wpmobilebot commented Nov 7, 2022 •

edited

Loading

Uh oh!

zwarm left a comment •

edited

Loading

Uh oh!

peril-wordpress-mobile bot commented Nov 17, 2022 •

edited

Loading

Uh oh!

AjeshRPai commented Nov 17, 2022

Uh oh!

shaunandrews commented Nov 18, 2022

Uh oh!

enejb commented Nov 18, 2022

Uh oh!

AjeshRPai commented Nov 21, 2022 •

edited

Loading

Uh oh!

zwarm left a comment

Uh oh!

osullivanchris commented Nov 21, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

Conversation

AjeshRPai commented Nov 7, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Known issues and Improvements

Testing Instructions

1. Make sure that the Overlay is not shown

2. Jetpack overlay is shown when Phase one is started

Stats

Notifications feature

Reader

3. Make sure that the Unit tests logic is correct

Regression Notes

Uh oh!

wpmobilebot commented Nov 7, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wpmobilebot commented Nov 7, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zwarm left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

peril-wordpress-mobile bot commented Nov 17, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AjeshRPai commented Nov 17, 2022

Uh oh!

shaunandrews commented Nov 18, 2022

Uh oh!

enejb commented Nov 18, 2022

Uh oh!

AjeshRPai commented Nov 21, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zwarm left a comment

Choose a reason for hiding this comment

Uh oh!

osullivanchris commented Nov 21, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

AjeshRPai commented Nov 7, 2022 •

edited

Loading

wpmobilebot commented Nov 7, 2022 •

edited

Loading

wpmobilebot commented Nov 7, 2022 •

edited

Loading

zwarm left a comment •

edited

Loading

peril-wordpress-mobile bot commented Nov 17, 2022 •

edited

Loading

AjeshRPai commented Nov 21, 2022 •

edited

Loading