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

Add editor onboarding capability #3437

Merged
merged 4 commits into from
May 13, 2021
Merged

Conversation

dcalhoun
Copy link
Member

@dcalhoun dcalhoun commented Apr 29, 2021

Add editor onboarding capability to control visibility of the feature. The flag combines (A) if the user has previously launched the Gutenberg editor and (B) if the user ID falls within the percentage of users allowed.

This flag is the first task of several to control when the editor onboarding is displayed. This flag will be used in combination with other, future flags (e.g. tooltipsSeen tracking which tips have been shown) to determine what should be displayed.

// If the user is eligible to begin onboarding...
if (canViewEditorOnboarding) {
  // Display first tooltip
}

// The user began onboarding, but has not yet seen the second tooltip...
if (tooltipsSeen.length > 1 && !tooltipsSeen.includes('tooltip2')) {
  // Display second tooltip
}

To test:

As this PR and its siblings contain no user-facing changes, the following test plan involves logging out the newly added capability to ensure it is working properly. The value behind shipping this addition on its own is to (A) begin tracking editor launches sooner rather than later and (B) produce smaller code diffs to review.

Patch to log editor onboarding flag
diff --git a/packages/block-editor/src/components/inserter/index.native.js b/packages/block-editor/src/components/inserter/index.native.js
index 463e03c116..9c3a023466 100644
--- a/packages/block-editor/src/components/inserter/index.native.js
+++ b/packages/block-editor/src/components/inserter/index.native.js
@@ -325,7 +325,12 @@ export default compose( [
 
 			const {
 				__experimentalShouldInsertAtTheTop: shouldInsertAtTheTop,
+				canViewEditorOnboarding,
 			} = getSettings();
+			console.log(
+				'>>> canViewEditorOnboarding',
+				canViewEditorOnboarding
+			);
 
 			// if post title is selected insert as first block
 			if ( shouldInsertAtTheTop ) {
  1. Create a patch file on your local system from the diff above.
  2. Clone this gutenberg-mobile branch.
  3. cd gutenberg && git apply path/to/patch-file.diff && cd ..
  4. Run the Metro server.
  5. Clone the corresponding branches for WPAndroid and WPiOS, run the main apps leveraging this branch via the integration guides for WPAndroid and WPiOS respectively.
  6. Launch the editor.
  7. Expected: >>> canViewEditorOnboarding true is logged in the Metro server as it is the first time the editor has launched.
  8. Close the editor.
  9. Launch the editor.
  10. Expected: >>> canViewEditorOnboarding false is logged in the Metro server for all editor launches moving forward.

PR submission checklist:

  • I have considered adding unit tests where possible.
  • I have considered if this change warrants user-facing release notes more info and have added them to RELEASE-NOTES.txt if necessary.

@peril-wordpress-mobile
Copy link

peril-wordpress-mobile bot commented May 4, 2021

Wanna run full suite of Android and iOS UI tests? Click here and 'Approve' CI job!

@dcalhoun dcalhoun self-assigned this May 6, 2021
@dcalhoun dcalhoun removed the [Status] DO NOT MERGE Do not merge this PR label May 6, 2021
@dcalhoun dcalhoun requested a review from jhnstn May 6, 2021 20:53
@dcalhoun
Copy link
Member Author

dcalhoun commented May 6, 2021

@jhnstn I debated as to whether we should attempt to merge these changes on their own or combine them with the first user-facing feature (i.e. the “Tap to add content” tooltip). I ultimately decided to proceed with opening a PR as-is. Because of this, the test plan is a bit more manual. I figured it might be better to receive your feedback sooner rather than later and also keep the code review focused on a smaller amount of changes.

Let me know if believe it makes more sense to wait to merge this work until we complete the first tooltip. Regardless, I'd still appreciate your input on the approach thus far. Thanks!

@dcalhoun dcalhoun marked this pull request as ready for review May 6, 2021 22:58
@jhnstn
Copy link
Member

jhnstn commented May 11, 2021

@dcalhoun I like shipping this change now before we get into the UI work.

Copy link
Member

@jhnstn jhnstn left a comment

Choose a reason for hiding this comment

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

Gutenberg PR approved 👍

@dcalhoun dcalhoun added this to the 1.53.0 (17.4) milestone May 12, 2021
@dcalhoun dcalhoun force-pushed the add/editor-onboarding-capability branch from 39653d4 to 1404414 Compare May 13, 2021 18:18
@dcalhoun dcalhoun merged commit cc970bb into develop May 13, 2021
@dcalhoun dcalhoun deleted the add/editor-onboarding-capability branch May 13, 2021 20:39
@dcalhoun dcalhoun mentioned this pull request May 13, 2021
2 tasks
@AmandaRiu AmandaRiu mentioned this pull request May 14, 2021
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Enhancement Improves a current area of the editor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants