Nv 2099 new in app quick start sandbox screen #3273

BiswaViraj · 2023-04-22T12:29:15Z

What change does this PR introduce?

adds the new in-app onboarding flow

Why was this change needed?

Other information (Screenshots)

Screen.Recording.2023-04-22.at.6.12.16.PM.mov

linear · 2023-04-22T12:29:18Z

NV-2099 new in app quick start sandbox screen implementation

BiswaViraj · 2023-04-22T12:30:19Z

apps/web/src/components/quick-start/digest-demo-flow/DigestDemoFlow.tsx

moved the existing files to the digest-demo-flow folder

BiswaViraj · 2023-04-22T12:30:39Z

apps/web/src/components/quick-start/digest-demo-flow/DigestDemoFlowProvider.tsx

moved the existing files to the digest-demo-flow folder

BiswaViraj · 2023-04-22T12:31:07Z

apps/web/src/components/quick-start/digest-demo-flow/DigestNode.tsx

moved the existing files to the digest-demo-flow folder and renamed the NodeStep file

BiswaViraj · 2023-04-22T12:31:18Z

apps/web/src/components/quick-start/digest-demo-flow/EmailNode.tsx

moved the existing files to the digest-demo-flow folder

BiswaViraj · 2023-04-22T12:32:01Z

apps/web/src/components/quick-start/digest-demo-flow/NodeStepWithPopover.tsx

Renamed the file and extracted the Node to a separate component

LetItRock

hey, awesome work! 🙌

I've found a few problems:

When I've clicked the trigger a couple of times I do see the horizontal scrollbar
If I refresh the page, they disappear but I do see an unread notifications count:
When I do open the notification center the notifications do disappear:

Screen.Recording.2023-04-25.at.12.54.15.mov

When I had some notifications in the notification center and I do open it, they are appearing in the sandbox:

Screen.Recording.2023-04-25.at.11.19.40.mov

The notification center is jumping on smaller screens.

Screen.Recording.2023-04-25.at.11.17.41.mov

The sandbox content is not centered.

apps/web/src/pages/quick-start/steps/InAppSuccess.tsx

apps/web/src/components/quick-start/in-app-onboarding/InAppSandbox.tsx

LetItRock · 2023-04-25T11:25:19Z

apps/web/src/components/quick-start/in-app-onboarding/InAppSandbox.tsx

+          <Ellipses isTransparent={false} />
+          <div>Your app or Demo app</div>
+          <Ellipses isTransparent={true} />


you don't need to draw transparent ellipses at the end, just position the title appropriately with css, like: margin: 0 auto; should work...
then you won't need that isTransparent flag at all

Tend to agree I think we could remove the last one with isTransparent={true}

The title needs to be at the center of the whole row, not just the center of available/remaining space.
margin: 0 auto will center it only for the available space.
for example, if the row width is 200px, and the ellipse takes 50px of width space, then margin: 0 auto will center the title of the available 150px; which isn't the center of the Row.

To center it I can think of a couple of ways,
The first one will be to have the title align center with width: 100%, and position the ellipse at the left using absolute: position (not fan of absolute positioning),
and the other one is the one that I've implemented.

LetItRock · 2023-04-25T11:25:31Z

apps/web/src/components/quick-start/in-app-onboarding/InAppSandbox.tsx

+            <Ellipses isTransparent={false} />
+            <div>Novu UI</div>
+            <Ellipses isTransparent={true} />


apps/web/src/components/quick-start/in-app-onboarding/TriggerNode.tsx

apps/web/src/components/quick-start/in-app-onboarding/InAppSandboxWorkflow.tsx

apps/web/src/pages/quick-start/components/QuickStartWrapper.tsx

apps/web/src/pages/quick-start/components/SetupStatus.tsx

LetItRock · 2023-04-25T12:17:42Z

apps/web/src/App.tsx

                  <Route path={ROUTES.QUICK_START_NOTIFICATION_CENTER} element={<NotificationCenter />} />
                  <Route path={ROUTES.QUICK_START_SETUP} element={<FrameworkSetup />} />
                  <Route path={ROUTES.QUICK_START_SETUP_FRAMEWORK} element={<Setup />} />
-                  <Route path={ROUTES.QUICK_START_SETUP_TRIGGER} element={<Trigger />} />
+                  <Route path={ROUTES.QUICK_START_SETUP_SUCCESS} element={<InAppSuccess />} />


it would be nice to see e2e tests for the in-app onboarding

Will add them in a different PR :)

ainouzgali · 2023-05-03T08:21:10Z

Looks great @BiswaViraj, a few things I've noticed:
When opening my notification center bell, it shows the notifications I already have in the sandbox. Also, shows the unseen count badge in both locations - in my notification center, shows the count badge but not the notifications.
Also, the go back button is not working from /quickstart/notification-center

djabarovgeorge · 2023-05-03T08:27:25Z

apps/web/src/components/quick-start/in-app-onboarding/InAppSandbox.tsx

+          <Ellipses isTransparent={false} />
+          <div>Your app or Demo app</div>
+          <Ellipses isTransparent={true} />


Tend to agree I think we could remove the last one with isTransparent={true}

djabarovgeorge · 2023-05-03T08:48:43Z

apps/web/src/pages/quick-start/steps/NotificationCenter.tsx

+  flex: 1;
+  height: 100%;
+  width: 100%;
+  position: absolute;


The go back button on the demo sandbox is not clickable I think it is because of the position:absolute; here

Yeah it was 😅, fixed it :D

ainouzgali

Great work @BiswaViraj ! Just noted very small things

Let's merge it and start testing 🚀

About the subscriber, I'm not sure that actually creating the subscriber for them is optimal scenario. As it actually adds that onboarding subscriber to their organization - but its only in dev, so probably its ok.

apps/web/src/components/quick-start/in-app-onboarding/InAppSandbox.tsx

apps/web/src/components/quick-start/common/Workflow.styles.tsx

ainouzgali · 2023-05-08T10:50:00Z

apps/web/src/components/quick-start/in-app-onboarding/TriggerNode.tsx

+    if (!templatesLoading && !notificationGroupLoading && !createTemplateLoading && !onboardingNotificationTemplate) {
+      createOnBoardingTemplate();
+    }
+  }, [templates, onboardingNotificationTemplate]);


Why are we getting all templates? Can't we get only that specific one?

I am curious about this one

apps/web/src/design-system/icons/general/Discord.tsx

apps/web/src/pages/quick-start/components/QuickStartWrapper.tsx

apps/web/src/pages/quick-start/components/SetupStatus.tsx

ainouzgali · 2023-05-08T13:54:46Z

Is it a missing light theme change?

BiswaViraj self-assigned this Apr 22, 2023

github-actions bot added the @novu/web label Apr 22, 2023

BiswaViraj commented Apr 22, 2023

View reviewed changes

BiswaViraj requested review from scopsy and djabarovgeorge April 22, 2023 12:35

LetItRock reviewed Apr 25, 2023

View reviewed changes

BiswaViraj force-pushed the nv-2099-new-in-app-quick-start-sandbox-screen branch from 5e9b37f to 4bf3f8a Compare May 2, 2023 16:27

github-actions bot added the @novu/notification-center label May 2, 2023

djabarovgeorge reviewed May 3, 2023

View reviewed changes

BiswaViraj force-pushed the nv-2099-new-in-app-quick-start-sandbox-screen branch 2 times, most recently from fdec8af to f95c7de Compare May 4, 2023 10:39

BiswaViraj requested review from djabarovgeorge, LetItRock, ainouzgali, p-fernandez and jainpawan21 May 4, 2023 10:52

ainouzgali approved these changes May 8, 2023

View reviewed changes

BiswaViraj added 9 commits May 9, 2023 17:50

feat: sandbox screen

8b175c8

chore: moved files to folder

f4d367a

feat: framework selector screen

cc3e2f0

feat: success screen

863c792

feat: instructions screen

6c7019e

refactor: removed unused imports

b08bfed

feat: redirect to create workflow

7f1058f

faet: trigger functionality

d70c950

refactor: removed unused code

eea3e92

BiswaViraj added 12 commits May 9, 2023 17:50

refactor: removed unused file

885a786

feat: added sandbox subscriber

0f3836e

fix: header section width issue

916c78d

feat: added analytics

7360ed0

fix: CI errors

c0351be

fix: save succes screen to current onboarding step store

121193f

fix: allow multiple instances of nc, and other changes

ea4a0ee

fix: Deepsource error, and UI align

f4bb345

fix: deepsource error

4ef8006

fix: import error

1a0ef53

fix: back button not clickable

74ed9e6

fix: after pr comments

3eb037b

BiswaViraj force-pushed the nv-2099-new-in-app-quick-start-sandbox-screen branch from 752c2da to 3eb037b Compare May 9, 2023 13:44

BiswaViraj added this pull request to the merge queue May 9, 2023

Merged via the queue into next with commit 6de5759 May 9, 2023
26 checks passed

BiswaViraj deleted the nv-2099-new-in-app-quick-start-sandbox-screen branch May 9, 2023 14:27

BiswaViraj restored the nv-2099-new-in-app-quick-start-sandbox-screen branch May 10, 2023 06:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Nv 2099 new in app quick start sandbox screen #3273

Nv 2099 new in app quick start sandbox screen #3273

BiswaViraj commented Apr 22, 2023 •

edited

linear bot commented Apr 22, 2023

BiswaViraj Apr 22, 2023

BiswaViraj Apr 22, 2023

BiswaViraj Apr 22, 2023

BiswaViraj Apr 22, 2023

BiswaViraj Apr 22, 2023

LetItRock left a comment

LetItRock Apr 25, 2023

djabarovgeorge May 3, 2023

BiswaViraj May 4, 2023

LetItRock Apr 25, 2023

LetItRock Apr 25, 2023

BiswaViraj May 4, 2023

ainouzgali commented May 3, 2023

djabarovgeorge May 3, 2023

djabarovgeorge May 3, 2023

BiswaViraj May 4, 2023

ainouzgali left a comment

ainouzgali May 8, 2023

ainouzgali May 8, 2023

ainouzgali commented May 8, 2023

Nv 2099 new in app quick start sandbox screen #3273

Nv 2099 new in app quick start sandbox screen #3273

Conversation

BiswaViraj commented Apr 22, 2023 • edited

What change does this PR introduce?

Why was this change needed?

Other information (Screenshots)

linear bot commented Apr 22, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LetItRock left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ainouzgali commented May 3, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ainouzgali left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ainouzgali commented May 8, 2023

BiswaViraj commented Apr 22, 2023 •

edited