feat: notification modal opening logic #4132

ilasw · 2025-01-30T13:03:37Z

Changes

Requires dailydotdev/daily-api#2632

Cleaned coming soon experiment so context is not required anymore
Added modal (+Query for gifter user)
BootPopover logic for gifted plus
Added mutation for disabling gift popover at close

Events

Did you introduce any new tracking events?

Experiment

Did you introduce any new experiments?

Manual Testing

Caution

Please make sure existing components are not breaking/affected by this PR

Preview domain

https://mi-753-fe-notification-modal-ope.preview.app.daily.dev

…apps into MI-746-gifting-plus

vercel · 2025-01-30T13:03:41Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Updated (UTC)
daily-webapp	✅ Ready (Inspect)	Visit Preview	Feb 3, 2025 4:36pm

1 Skipped Deployment

Name	Status	Preview	Updated (UTC)
storybook	⬜️ Ignored (Inspect)		Feb 3, 2025 4:36pm

capJavert · 2025-01-30T15:33:09Z

packages/shared/src/components/modals/BootPopups.tsx

+   * Received gift plus modal
+   */
+  useEffect(() => {
+    if (!alerts?.shouldShowGiftPlus || !user.isPlus) {


don't we use boot for this instead of alerts or just was not updated yet?

I added it inside the alerts object as we did for shouldShowFeedFeedback

https://github.com/dailydotdev/daily-api/blob/main/src/routes/boot.ts#L542

you should use it directly on user from useAuthContext, it will be confusing to put something that is not an alert into alerts object, it can also create invalid gql calls to alerts update mutation (because that field does not exist there)

my bad I completely missed you added it there in the API PR

can you adjust that on API? @ilasw

Okay, so I've to add to the graphorm user object? @capJavert

nvm added to user in boot

sshanzel · 2025-01-30T17:12:15Z

packages/shared/src/components/modals/BootPopups.tsx

+          updateUserProfile({
+            flags: { showPlusGift: false },
+          });


Do we have to wait for the user to close it before we turn off the flag?

I don't think this is mandatory to wait plus don't return any promise.
But if you think is important I can change this behaviour 👀

Maybe it is more of how we should be doing it generally for our modals. Anyway, shouldn't be a blocker and we can likely keep it.

packages/shared/src/components/modals/common.tsx

packages/shared/src/components/plus/GiftReceivedPlusModal.tsx

sshanzel · 2025-01-30T17:17:02Z

packages/shared/src/components/plus/GiftReceivedPlusModal.tsx

+          />
+          <PlusList className="overflow-clip !py-0" />
+        </div>
+        <div className="flex flex-col gap-4 tablet:hidden">


I think we don't need this footer at all. We should just use the drawer's close button. The issue is, I don't think it should actually be called Cancel. It should rather be called Close.

We have it in design, should we drop it?

We should probably check with design/product team.

Updated ✔️

packages/shared/src/components/plus/GiftReceivedPlusModal.tsx

sshanzel

Looks amazing! Don't have anything major going on except the Close button on mobile.

Co-authored-by: Lee Hansel Solevilla <[email protected]>

…github.com/dailydotdev/apps into MI-753-fe-notification-modal-opening-logic

…opening-logic

sshanzel · 2025-01-31T14:11:37Z

packages/shared/src/components/plus/GiftReceivedPlusModal.tsx

+          <Button
+            type="button"
+            onClick={onRequestClose}
+            variant={ButtonVariant.Float}
+          >
+            Close
+          </Button>


We should not need this close button after removing the props below:

apps/packages/shared/src/components/plus/GiftReceivedPlusModal.tsx

Lines 79 to 81 in f5333b9

drawerProps={{

displayCloseButton: false,

}}

sshanzel

Just one last thing though everything looked amazing already, hence approving 🚀

capJavert

Code wise don't see anything blocking, nice work! Few comments for improvement.

capJavert · 2025-01-31T17:04:46Z

packages/shared/src/lib/user.ts

+  flags?: {
+    showPlusGift?: boolean;
+  };


Suggested change

flags?: {

showPlusGift?: boolean;

};

flags?: UserFlagsPublic;

capJavert · 2025-01-31T17:05:25Z

packages/shared/src/hooks/useProfileForm.ts

+export type UserFlagsPublic = {
+  showPlusGift: boolean;
+};


Suggested change

export type UserFlagsPublic = {

showPlusGift: boolean;

};

export type UserFlagsPublic = Partial<{

showPlusGift: boolean;

}>;

flags are always optional so I think we can jsut do this on the type

then below Partial can be removed

capJavert · 2025-01-31T17:08:25Z

packages/shared/src/components/plus/GiftReceivedPlusModal.tsx

+  if (!gifter || isLoading) {
+    return null;
+  }


Maybe add a modal like loader so user knows something is opening, see https://github.com/dailydotdev/apps/pull/4128/files#diff-ea1df30479911bb7e3f5fcb0727c06eac149de4aa7f08e1dae88b4ce5c217a9cR297-R303

non blocking

sshanzel · 2025-02-04T10:58:38Z

@capJavert I've now applied the non-blocking changes 🙏 will merge soon 🙌

capJavert · 2025-02-04T11:30:13Z

Thanks @sshanzel !

ilasw added 2 commits January 30, 2025 14:02

feat: init GiftReceivedPlusModal

6492acc

Merge branch 'MI-746-gifting-plus' of https://github.com/dailydotdev/…

412af11

…apps into MI-746-gifting-plus

ilasw changed the base branch from main to MI-746-gifting-plus January 30, 2025 13:03

vercel bot deployed to Preview – daily-webapp January 30, 2025 13:06 View deployment

feat: add update flag on modal close

118aba6

vercel bot deployed to Preview – daily-webapp January 30, 2025 14:02 View deployment

feat: add query for gifter

c27456d

vercel bot deployed to Preview – daily-webapp January 30, 2025 15:04 View deployment

fix: chunk name dupe

f9116f1

vercel bot deployed to Preview – daily-webapp January 30, 2025 15:16 View deployment

ilasw added 2 commits January 30, 2025 16:24

refactor: clean coming soon experiment

6bb2e44

refactor: better component structure

448dba4

ilasw marked this pull request as ready for review January 30, 2025 15:25

ilasw requested a review from a team as a code owner January 30, 2025 15:25

ilasw requested review from rebelchris, omBratteng, capJavert, sshanzel, nensidosari and AmarTrebinjac January 30, 2025 15:25

vercel bot deployed to Preview – daily-webapp January 30, 2025 15:27 View deployment

ilasw mentioned this pull request Jan 30, 2025

feat: add query for get gifter data dailydotdev/daily-api#2632

Merged

capJavert reviewed Jan 30, 2025

View reviewed changes

refactor: moved shouldShowGiftPlus into user object

6bb44dd

vercel bot deployed to Preview – daily-webapp January 30, 2025 16:38 View deployment

sshanzel reviewed Jan 30, 2025

View reviewed changes

packages/shared/src/components/modals/common.tsx Outdated Show resolved Hide resolved

sshanzel reviewed Jan 30, 2025

View reviewed changes

packages/shared/src/components/plus/GiftReceivedPlusModal.tsx Outdated Show resolved Hide resolved

sshanzel reviewed Jan 30, 2025

View reviewed changes

packages/shared/src/components/plus/GiftReceivedPlusModal.tsx Outdated Show resolved Hide resolved

sshanzel reviewed Jan 30, 2025

View reviewed changes

ilasw and others added 3 commits January 30, 2025 18:46

fix: missing type in button

6f3fd38

Co-authored-by: Lee Hansel Solevilla <[email protected]>

fix: remove space

df89876

Co-authored-by: Lee Hansel Solevilla <[email protected]>

fix: use camelCase

9844fb6

Co-authored-by: Lee Hansel Solevilla <[email protected]>

vercel bot deployed to Preview – daily-webapp January 30, 2025 17:50 View deployment

ilasw added 2 commits January 31, 2025 09:53

refactor: moved showGiftPlus into user flags

f5eda74

Merge branch 'MI-753-fe-notification-modal-opening-logic' of https://…

c88240f

…github.com/dailydotdev/apps into MI-753-fe-notification-modal-opening-logic

vercel bot deployed to Preview – daily-webapp January 31, 2025 09:02 View deployment

ilasw added 2 commits January 31, 2025 10:58

Merge branch 'MI-746-gifting-plus' into MI-753-fe-notification-modal-…

1173138

…opening-logic

fix: typo change

e0ed0a6

ilasw requested review from sshanzel and capJavert January 31, 2025 10:00

vercel bot deployed to Preview – daily-webapp January 31, 2025 10:02 View deployment

Merge branch 'MI-746-gifting-plus' into MI-753-fe-notification-modal-…

f5333b9

…opening-logic

vercel bot deployed to Preview – daily-webapp January 31, 2025 12:31 View deployment

sshanzel reviewed Jan 31, 2025

View reviewed changes

sshanzel approved these changes Jan 31, 2025

View reviewed changes

capJavert approved these changes Jan 31, 2025

View reviewed changes

sshanzel added 2 commits February 4, 2025 00:29

fix: modal ctas

6421883

fix: missing loader

9d299e8

vercel bot deployed to Preview – daily-webapp February 3, 2025 16:33 View deployment

fix: modal

32a3af5

vercel bot deployed to Preview – daily-webapp February 3, 2025 16:36 View deployment

sshanzel merged commit cef485d into MI-746-gifting-plus Feb 4, 2025
10 checks passed

sshanzel deleted the MI-753-fe-notification-modal-opening-logic branch February 4, 2025 11:24

sshanzel mentioned this pull request Feb 7, 2025

feat: add gifting plus entrypoint and page #4121

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: notification modal opening logic #4132

feat: notification modal opening logic #4132

ilasw commented Jan 30, 2025 •

edited

Loading

vercel bot commented Jan 30, 2025 •

edited

Loading

capJavert Jan 30, 2025

ilasw Jan 30, 2025

capJavert Jan 30, 2025 •

edited

Loading

capJavert Jan 30, 2025

capJavert Jan 30, 2025

ilasw Jan 30, 2025

ilasw Jan 30, 2025

sshanzel Jan 30, 2025

ilasw Jan 30, 2025 •

edited

Loading

sshanzel Jan 31, 2025

sshanzel Jan 30, 2025

ilasw Jan 30, 2025

sshanzel Jan 31, 2025

ilasw Jan 31, 2025

sshanzel left a comment

sshanzel Jan 31, 2025

sshanzel left a comment

capJavert left a comment

capJavert Jan 31, 2025

capJavert Jan 31, 2025

capJavert Jan 31, 2025

capJavert Jan 31, 2025

capJavert Jan 31, 2025

sshanzel commented Feb 4, 2025

capJavert commented Feb 4, 2025

feat: notification modal opening logic #4132

feat: notification modal opening logic #4132

Conversation

ilasw commented Jan 30, 2025 • edited Loading

Changes

Events

Experiment

Manual Testing

Preview domain

vercel bot commented Jan 30, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

capJavert Jan 30, 2025 • edited Loading

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

ilasw Jan 30, 2025 • edited Loading

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

sshanzel left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sshanzel left a comment

Choose a reason for hiding this comment

capJavert 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

sshanzel commented Feb 4, 2025

capJavert commented Feb 4, 2025

ilasw commented Jan 30, 2025 •

edited

Loading

vercel bot commented Jan 30, 2025 •

edited

Loading

capJavert Jan 30, 2025 •

edited

Loading

ilasw Jan 30, 2025 •

edited

Loading