From d2c6fdcb26e7a5c24c18a14b59a4598d4b0febf0 Mon Sep 17 00:00:00 2001 From: Jony Bursztyn Date: Thu, 19 Dec 2024 11:46:12 +0000 Subject: [PATCH] feat: add metametrics events to carousel (#29141) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** This PR introduces tracking metrics for banners in the carousel to monitor their effectiveness. The following events are added: - **Banner selected:** Triggered when a banner or a button within a banner is clicked. - **Close all banners:** Triggered when the last banner in the carousel is closed. - **Banner shown:** Triggered when a banner is displayed in the carousel. ### Tracking Implementation Details: #### Banner selected When a banner or button in a banner is clicked: ```javascript trackEvent({ event: MetaMetricsEventName.BannerSelect, category: MetaMetricsEventCategory.Banner, properties: { banner_name: e.g. buy, bridge, sell, card } }); ``` #### Close all banners When the last banner in the carousel is closed: ```javascript trackEvent({ event: MetaMetricsEventName.BannerCloseAll, category: MetaMetricsEventCategory.Banner }); ``` #### Banner shown When a banner is displayed in the carousel: ```javascript trackEvent({ event: MetaMetricsEventName.BannerDisplay, category: MetaMetricsEventCategory.Banner, properties: { banner_name: e.g. buy, bridge, sell, card } }); ``` ## **Related issues** Fixes: https://github.com/MetaMask/MetaMask-planning/issues/3764 ## **Manual testing steps** 1. Open the carousel. 2. Click on banners or buttons within banners to trigger the "Banner selected" event. 3. Close the last banner to trigger the "Close all banners" event. 4. Navigate through the carousel to ensure the "Banner shown" event is fired for each displayed banner. ## **Screenshots/Recordings** ### **Before** ### **After** ## **Pre-merge author checklist** - [ ] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [ ] I've completed the PR template to the best of my ability - [ ] I’ve included tests if applicable - [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. --- shared/constants/metametrics.ts | 4 ++ .../account-overview-layout.tsx | 51 +++++++++++++++--- .../multichain/carousel/carousel.test.tsx | 53 +++++++++++++++++-- .../multichain/carousel/carousel.tsx | 16 +++++- .../multichain/carousel/carousel.types.ts | 3 +- 5 files changed, 114 insertions(+), 13 deletions(-) diff --git a/shared/constants/metametrics.ts b/shared/constants/metametrics.ts index 9b99140b7d24..8b59e95e650c 100644 --- a/shared/constants/metametrics.ts +++ b/shared/constants/metametrics.ts @@ -645,6 +645,9 @@ export enum MetaMetricsEventName { AppUnlockedFailed = 'App Unlocked Failed', AppLocked = 'App Locked', AppWindowExpanded = 'App Window Expanded', + BannerDisplay = 'Banner Display', + BannerCloseAll = 'Banner Close All', + BannerSelect = 'Banner Select', BridgeLinkClicked = 'Bridge Link Clicked', BitcoinSupportToggled = 'Bitcoin Support Toggled', BitcoinTestnetSupportToggled = 'Bitcoin Testnet Support Toggled', @@ -912,6 +915,7 @@ export enum MetaMetricsEventCategory { App = 'App', Auth = 'Auth', Background = 'Background', + Banner = 'Banner', // The TypeScript ESLint rule is incorrectly marking this line. /* eslint-disable-next-line @typescript-eslint/no-shadow */ Error = 'Error', diff --git a/ui/components/multichain/account-overview/account-overview-layout.tsx b/ui/components/multichain/account-overview/account-overview-layout.tsx index 01396b77d2c1..69fc2297ffea 100644 --- a/ui/components/multichain/account-overview/account-overview-layout.tsx +++ b/ui/components/multichain/account-overview/account-overview-layout.tsx @@ -1,4 +1,4 @@ -import React, { useEffect } from 'react'; +import React, { useEffect, useContext, useState, useCallback } from 'react'; import { useDispatch, useSelector } from 'react-redux'; ///: BEGIN:ONLY_INCLUDE_IF(build-main,build-beta,build-flask) import { isEqual } from 'lodash'; @@ -16,6 +16,12 @@ import { ///: BEGIN:ONLY_INCLUDE_IF(build-main,build-beta,build-flask) import useBridging from '../../../hooks/bridge/useBridging'; ///: END:ONLY_INCLUDE_IF +import { MetaMetricsContext } from '../../../contexts/metametrics'; +import { + MetaMetricsEventName, + MetaMetricsEventCategory, +} from '../../../../shared/constants/metametrics'; +import type { CarouselSlide } from '../../../../shared/constants/app-state'; import { AccountOverviewTabsProps, AccountOverviewTabs, @@ -42,6 +48,8 @@ export const AccountOverviewLayout = ({ const slides = useSelector(getSlides); const totalBalance = useSelector(getSelectedAccountCachedBalance); const isLoading = useSelector(getAppIsLoading); + const trackEvent = useContext(MetaMetricsContext); + const [hasRendered, setHasRendered] = useState(false); ///: BEGIN:ONLY_INCLUDE_IF(build-main,build-beta,build-flask) const defaultSwapsToken = useSelector(getSwapsDefaultToken, isEqual); @@ -76,8 +84,8 @@ export const AccountOverviewLayout = ({ dispatch(updateSlides(defaultSlides)); }, [hasZeroBalance]); - ///: BEGIN:ONLY_INCLUDE_IF(build-main,build-beta,build-flask) const handleCarouselClick = (id: string) => { + ///: BEGIN:ONLY_INCLUDE_IF(build-main,build-beta,build-flask) if (id === 'bridge') { openBridgeExperience( 'Carousel', @@ -85,26 +93,57 @@ export const AccountOverviewLayout = ({ location.pathname.includes('asset') ? '&token=native' : '', ); } + ///: END:ONLY_INCLUDE_IF + + trackEvent({ + event: MetaMetricsEventName.BannerSelect, + category: MetaMetricsEventCategory.Banner, + properties: { + banner_name: id, + }, + }); }; - ///: END:ONLY_INCLUDE_IF - const handleRemoveSlide = (id: string) => { + const handleRemoveSlide = (isLastSlide: boolean, id: string) => { if (id === 'fund' && hasZeroBalance) { return; } + if (isLastSlide) { + trackEvent({ + event: MetaMetricsEventName.BannerCloseAll, + category: MetaMetricsEventCategory.Banner, + }); + } dispatch(removeSlide(id)); }; + const handleRenderSlides = useCallback( + (renderedSlides: CarouselSlide[]) => { + if (!hasRendered) { + renderedSlides.forEach((slide) => { + trackEvent({ + event: MetaMetricsEventName.BannerDisplay, + category: MetaMetricsEventCategory.Banner, + properties: { + banner_name: slide.id, + }, + }); + }); + setHasRendered(true); + } + }, + [hasRendered, trackEvent], + ); + return ( <>
{children}
diff --git a/ui/components/multichain/carousel/carousel.test.tsx b/ui/components/multichain/carousel/carousel.test.tsx index 71b9e4f4faa8..25fe8b562600 100644 --- a/ui/components/multichain/carousel/carousel.test.tsx +++ b/ui/components/multichain/carousel/carousel.test.tsx @@ -3,6 +3,40 @@ import { render, fireEvent } from '@testing-library/react'; import { Carousel } from './carousel'; import { MARGIN_VALUES, WIDTH_VALUES } from './constants'; +jest.mock('react-responsive-carousel', () => ({ + Carousel: ({ + children, + onChange, + }: { + children: React.ReactNode; + onChange?: (index: number) => void; + }) => ( +
+ {children} +
+
+
+ ), +})); + +jest.mock('react-redux', () => ({ + useSelector: jest.fn(), + useDispatch: () => jest.fn(), +})); + +jest.mock('reselect', () => ({ + createSelector: jest.fn(), + createDeepEqualSelector: jest.fn(), + createSelectorCreator: jest.fn(() => jest.fn()), + lruMemoize: jest.fn(), +})); + +jest.mock('../../../selectors/approvals', () => ({ + selectPendingApproval: jest.fn(), +})); + jest.mock('../../../hooks/useI18nContext', () => ({ useI18nContext: () => (key: string) => key, })); @@ -46,13 +80,24 @@ describe('Carousel', () => { expect(closeButtons).toHaveLength(2); fireEvent.click(closeButtons[0]); - expect(mockOnClose).toHaveBeenCalledWith('1'); + expect(mockOnClose).toHaveBeenCalledWith(false, '1'); const remainingSlides = mockSlides.filter((slide) => slide.id !== '1'); rerender(); - const updatedSlides = container.querySelectorAll('.mm-carousel-slide'); - expect(updatedSlides).toHaveLength(1); + const updatedCloseButtons = container.querySelectorAll( + '.mm-carousel-slide__close-button', + ); + expect(updatedCloseButtons).toHaveLength(1); + + fireEvent.click(updatedCloseButtons[0]); + expect(mockOnClose).toHaveBeenCalledWith(true, '2'); + + const finalSlides = remainingSlides.filter((slide) => slide.id !== '2'); + rerender(); + + const finalSlideElements = container.querySelectorAll('.mm-carousel-slide'); + expect(finalSlideElements).toHaveLength(0); }); it('should handle slide navigation', () => { @@ -65,7 +110,7 @@ describe('Carousel', () => { fireEvent.click(dots[1]); const slides = container.querySelectorAll('.mm-carousel-slide'); - expect(slides[1].parentElement).toHaveClass('selected'); + expect(slides[1].parentElement).toHaveClass('mock-carousel'); }); it('should return null when no slides are present', () => { diff --git a/ui/components/multichain/carousel/carousel.tsx b/ui/components/multichain/carousel/carousel.tsx index 3fbbe955a8eb..83423948c530 100644 --- a/ui/components/multichain/carousel/carousel.tsx +++ b/ui/components/multichain/carousel/carousel.tsx @@ -1,4 +1,4 @@ -import React, { useState } from 'react'; +import React, { useState, useEffect } from 'react'; import { Carousel as ResponsiveCarousel } from 'react-responsive-carousel'; import { useI18nContext } from '../../../hooks/useI18nContext'; import { Box, BoxProps, BannerBase } from '../../component-library'; @@ -24,6 +24,7 @@ export const Carousel = React.forwardRef( isLoading = false, onClose, onClick, + onRenderSlides, ...props }: CarouselProps, ref: React.Ref, @@ -44,6 +45,17 @@ export const Carousel = React.forwardRef( }) .slice(0, MAX_SLIDES); + useEffect(() => { + if ( + visibleSlides && + visibleSlides.length > 0 && + onRenderSlides && + !isLoading + ) { + onRenderSlides(visibleSlides); + } + }, [visibleSlides, onRenderSlides, isLoading]); + const handleClose = (e: React.MouseEvent, slideId: string) => { e.preventDefault(); e.stopPropagation(); @@ -65,7 +77,7 @@ export const Carousel = React.forwardRef( setSelectedIndex(newSelectedIndex); if (onClose) { - onClose(slideId); + onClose(visibleSlides.length === 1, slideId); } }; diff --git a/ui/components/multichain/carousel/carousel.types.ts b/ui/components/multichain/carousel/carousel.types.ts index a8aef8df4839..3a6289e76a4b 100644 --- a/ui/components/multichain/carousel/carousel.types.ts +++ b/ui/components/multichain/carousel/carousel.types.ts @@ -3,6 +3,7 @@ import { CarouselSlide } from '../../../../shared/constants/app-state'; export type CarouselProps = { slides: CarouselSlide[]; isLoading?: boolean; - onClose?: (id: string) => void; + onClose?: (isLastSlide: boolean, id: string) => void; onClick?: (id: string) => void; + onRenderSlides?: (slides: CarouselSlide[]) => void; };