Skip to content

Jetpack Mobile: Scan - Introduce feature flag#13334

Merged
malinajirka merged 3 commits intodevelopfrom
issue/13326-introduce-scan-feature-flag
Nov 11, 2020
Merged

Jetpack Mobile: Scan - Introduce feature flag#13334
malinajirka merged 3 commits intodevelopfrom
issue/13326-introduce-scan-feature-flag

Conversation

@ashiagr
Copy link
Copy Markdown
Contributor

@ashiagr ashiagr commented Nov 10, 2020 •
edited
Loading

#13326

This PR introduces a local feature flag for the Scan feature. When the feature flag is enabled a "Scan" menu is shown on the My Site tab (currently irrespective of the site plan).

To test

Prerequisite: Ensure ENABLE_FEATURE_CONFIGURATION is enabled in the build.gradle for the build variant you're testing.

  1. Launch app and go to My Site tab
  2. Notice that "Scan" menu is hidden
  3. Go to My Site -> Avatar icon in the toolbar -> App Settings -> Test feature configuration
  4. Enable ScanFeatureConfig and click on "Restart app"
  5. Go back to My Site and notice "Scan" menu is shown

device-2020-11-10-105404

PR submission checklist:

  • I have considered adding unit tests where possible.
  • 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.

@ashiagr ashiagr self-assigned this Nov 10, 2020
@peril-wordpress-mobile
Copy link
Copy Markdown

peril-wordpress-mobile bot commented Nov 10, 2020 •
edited
Loading

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

@ashiagr ashiagr added this to the 16.2 milestone Nov 10, 2020
@peril-wordpress-mobile
Copy link
Copy Markdown

peril-wordpress-mobile bot commented Nov 10, 2020

You can test the changes on this Pull Request by downloading the APK here.

ParaskP7
ParaskP7 reviewed Nov 10, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@ParaskP7 ParaskP7 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Great job! 🌟

Tested the changes, I was able to see Scan row after enabling ScanFeatureConfig.

PS: The only thing I can point is that the fillColor on the scan vector asset is not white, just like the others are, especially since the resource name suggest it is white indeed. I understand that the tint color anyway changes from the style afterwards, but we might want to keep consistency on those vector icons. Wdyt Ashita?
PPS: Also, I believe this file can be formatted further.

@ashiagr
Copy link
Copy Markdown
Contributor Author

ashiagr commented Nov 10, 2020

Thanks for the review @ParaskP7 !

The only thing I can point is that the fillColor on the scan vector asset is not white, just like the others are, especially since the resource name suggest it is white indeed.

PPS: Also, I believe this file can be formatted further.

You're absolutely right, I grabbed a temporary scan icon, I plan to replace it when we have some Figma designs ready - it allows to directly export icons. Can we wait till then @ParaskP7 ? I'll add an action item in the feature tasks list. Wdyt?

@ashiagr ashiagr mentioned this pull request Nov 10, 2020
Closed
67 tasks
@ParaskP7
Copy link
Copy Markdown
Contributor

ParaskP7 commented Nov 10, 2020

You're absolutely right, I grabbed a temporary scan icon, I plan to replace it when we have some Figma designs ready - it allows to directly export icons. Can we wait till then @ParaskP7 ? I'll add an action item in the feature tasks list. Wdyt?

Of course! I didn't understood this is a temp icon indeed. An action item will suffice, thank YOU Ashita! 🌟

@malinajirka malinajirka self-assigned this Nov 11, 2020
malinajirka
malinajirka approved these changes Nov 11, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@malinajirka malinajirka left a comment

Choose a reason for hiding this comment

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

LGTM, and works as expected;). Thank you both!

@malinajirka malinajirka merged commit 35dd069 into develop Nov 11, 2020
@ashiagr ashiagr deleted the issue/13326-introduce-scan-feature-flag branch November 12, 2020 08:46
@ashiagr ashiagr changed the title Jetpack Section: Scan - Introduce feature flag Jetpack Mobile: Scan - Introduce feature flag Dec 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ParaskP7 ParaskP7 ParaskP7 left review comments

@malinajirka malinajirka malinajirka approved these changes

@zwarm zwarm Awaiting requested review from zwarm

Assignees

@ashiagr ashiagr

@malinajirka malinajirka

Labels

Jetpack [Type] Task

Projects

None yet

Milestone

16.2

Development

Successfully merging this pull request may close these issues.

3 participants

@ashiagr @ParaskP7 @malinajirka