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

New user progress guides #4716

Merged
merged 21 commits into from
Jul 4, 2024
Merged

New user progress guides #4716

merged 21 commits into from
Jul 4, 2024

Conversation

pfrazee
Copy link
Collaborator

@pfrazee pfrazee commented Jul 2, 2024

  • Implement progress guide listing
  • Implement progress guide milestone toasts
  • Implement progress guide controller and action capture
  • Implement progress guide state-persistence to preferences
  • Show the listing in interstitials (waiting on [D1X] Add interstitials, component tweaks, placeholders #4697)
  • Do an a11y pass
  • Finalize the guides

Web

CleanShot.2024-07-01.at.19.22.43.mp4

Mobile

(Interstitials showing progress not yet done in this recording)

CleanShot.2024-07-01.at.19.19.58.mp4

Copy link

render bot commented Jul 2, 2024

Copy link

github-actions bot commented Jul 2, 2024

Old size New size Diff
6.58 MB 6.6 MB 25.71 KB (0.38%)

@pfrazee pfrazee force-pushed the paul/progress-guides branch from 120b74e to 9c5420c Compare July 3, 2024 04:51
@pfrazee
Copy link
Collaborator Author

pfrazee commented Jul 3, 2024

Added feed interstitials for mobile:

CleanShot 2024-07-02 at 22 32 20@2x

if (guide?.isComplete) {
return
}
if (guide?.guide === 'like-10-and-follow-7') {
Copy link
Member

Choose a reason for hiding this comment

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

Nit idea: this is basically a reducer, I wonder if the params would be more extensible if we made this a discriminated union of reducer actions?

Could then extract this into a pure function for future extensibility and testing (if we ever need it). The result is used for side effects, so I think that could happen as a separate step instead of blending to two 🤔

Copy link
Member

Choose a reason for hiding this comment

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

This can come later too

@@ -185,6 +187,7 @@ export function StepFinished() {
setActiveStarterPack(undefined)
setHasCheckedForStarterPack(true)
setQueuedTour(TOURS.HOME)
startProgressGuide('like-10-and-follow-7')
Copy link
Member

Choose a reason for hiding this comment

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

Should we check the gate here in case we want to test entirely separate variations or do you want to keep it more centralized?

Comment on lines +70 to +74
startProgressGuide(guide: ProgressGuideName) {
if (!gate('new_user_progress_guide')) {
return
}
if (guide === 'like-10-and-follow-7') {
Copy link
Member

Choose a reason for hiding this comment

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

Should probably have some sort of check here to prevent clobbering an existing active guide

Copy link
Member

Choose a reason for hiding this comment

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

This can come later

<ProgressGuideContext.Provider value={activeProgressGuide}>
<ProgressGuideControlContext.Provider value={controls}>
{children}
<ProgressGuideToast
Copy link
Member

Choose a reason for hiding this comment

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

Any issues with these always rendering?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, fixed

@@ -352,3 +354,25 @@ export function SuggestedFeeds() {
</View>
)
}

export function ProgressGuide() {
Copy link
Member

Choose a reason for hiding this comment

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

This renders all the time, even if the guide isn't active

const controls = React.useMemo(() => {
return {
startProgressGuide(guide: ProgressGuideName) {
if (!gate('new_user_progress_guide') && false) {
Copy link
Member

Choose a reason for hiding this comment

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

Just reminding us to remove this when we're done testing

@@ -369,6 +374,7 @@ function Header({
starterPack: starterPack.uri,
count: dids.length,
})
captureAction(ProgressGuideAction.Follow, dids.length)
Copy link
Collaborator

Choose a reason for hiding this comment

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

oh subtle

@gaearon gaearon merged commit 0ed99b8 into main Jul 4, 2024
6 checks passed
@gaearon gaearon deleted the paul/progress-guides branch July 4, 2024 02:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants