Skip to content

Commit

Permalink
feat: add metametrics events to carousel (#29141)
Browse files Browse the repository at this point in the history
<!--
Please submit this PR as a draft initially.
Do not mark it as "Ready for review" until the template has been
completely filled out, and PR status checks have passed at least once.
-->

## **Description**

<!--
Write a short description of the changes included in this pull request,
also include relevant motivation and context. Have in mind the following
questions:
1. What is the reason for the change?
2. What is the improvement/solution?
-->

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: MetaMask/MetaMask-planning#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**

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

### **Before**

<!-- [screenshots/recordings] -->

### **After**

<!-- [screenshots/recordings] -->

## **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.
  • Loading branch information
jonybur authored Dec 19, 2024
1 parent 5855938 commit d06dad7
Show file tree
Hide file tree
Showing 5 changed files with 114 additions and 13 deletions.
4 changes: 4 additions & 0 deletions shared/constants/metametrics.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down Expand Up @@ -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',
Expand Down
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -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,
Expand All @@ -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);
Expand Down Expand Up @@ -76,35 +84,66 @@ 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',
defaultSwapsToken,
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 (
<>
<div className="account-overview__balance-wrapper">{children}</div>
<Carousel
slides={slides}
isLoading={isLoading}
///: BEGIN:ONLY_INCLUDE_IF(build-main,build-beta,build-flask)
onClick={handleCarouselClick}
///: END:ONLY_INCLUDE_IF
onClose={handleRemoveSlide}
onRenderSlides={handleRenderSlides}
/>
<AccountOverviewTabs {...tabsProps}></AccountOverviewTabs>
</>
Expand Down
53 changes: 49 additions & 4 deletions ui/components/multichain/carousel/carousel.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}) => (
<div className="mock-carousel">
{children}
<div className="carousel-dots">
<button className="dot" onClick={() => onChange?.(1)} />
<button className="dot" onClick={() => onChange?.(0)} />
</div>
</div>
),
}));

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,
}));
Expand Down Expand Up @@ -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(<Carousel slides={remainingSlides} onClose={mockOnClose} />);

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(<Carousel slides={finalSlides} onClose={mockOnClose} />);

const finalSlideElements = container.querySelectorAll('.mm-carousel-slide');
expect(finalSlideElements).toHaveLength(0);
});

it('should handle slide navigation', () => {
Expand All @@ -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', () => {
Expand Down
16 changes: 14 additions & 2 deletions ui/components/multichain/carousel/carousel.tsx
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -24,6 +24,7 @@ export const Carousel = React.forwardRef(
isLoading = false,
onClose,
onClick,
onRenderSlides,
...props
}: CarouselProps,
ref: React.Ref<HTMLDivElement>,
Expand All @@ -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<HTMLElement>, slideId: string) => {
e.preventDefault();
e.stopPropagation();
Expand All @@ -65,7 +77,7 @@ export const Carousel = React.forwardRef(
setSelectedIndex(newSelectedIndex);

if (onClose) {
onClose(slideId);
onClose(visibleSlides.length === 1, slideId);
}
};

Expand Down
3 changes: 2 additions & 1 deletion ui/components/multichain/carousel/carousel.types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
};

0 comments on commit d06dad7

Please sign in to comment.