Skip to content

Commit 7e0f56e

Browse files
committed
[QF-3553] refactor
1 parent cdda764 commit 7e0f56e

File tree

9 files changed

+496
-37
lines changed

9 files changed

+496
-37
lines changed

src/components/Banner/Banner.module.scss

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@
2222

2323
.cta {
2424
padding-inline: var(--spacing-xsmall-px);
25-
padding-block: var(--spacing-xxxsmall-px);
25+
padding-block: var(--spacing-micro2-px);
2626
background-color: var(--color-success-faded);
2727
color: var(--color-text-link-new);
2828
font-size: var(--font-size-small-px);
@@ -38,7 +38,6 @@
3838

3939
.icon {
4040
> svg path {
41-
color: var(--color-text-link-new);
4241
fill: var(--color-text-link-new);
4342
}
4443
}

src/components/Banner/Banner.tsx

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
import { useCallback } from 'react';
2+
13
import styles from './Banner.module.scss';
24

35
import IconContainer, { IconColor, IconSize } from '@/dls/IconContainer/IconContainer';
@@ -21,22 +23,27 @@ const Banner = ({ text, ctaButtonText }: BannerProps) => {
2123
const { goal } = useGetStreakWithMetadata();
2224
const hasGoal = !!goal;
2325

24-
// Route logged-in users to progress page while loading to prevent jarring UX
25-
// if they have an existing goal. Falls back to reading-goal once loading completes
26-
// if no goal exists.
26+
// Route logged-in users with an existing goal to the progress page,
27+
// otherwise route to the reading-goal page.
2728
const ctaLink =
2829
isLoggedIn && hasGoal ? getReadingGoalProgressNavigationUrl() : getReadingGoalNavigationUrl();
2930

30-
const handleButtonClick = () => {
31+
const handleButtonClick = useCallback(() => {
3132
logButtonClick('banner_cta', {
3233
hasGoal,
3334
isLoggedIn,
3435
});
35-
};
36+
}, [hasGoal, isLoggedIn]);
3637

3738
return (
38-
<div className={styles.container} data-testid="banner">
39-
<p className={styles.text}>{text}</p>
39+
<div
40+
className={styles.container}
41+
data-testid="banner"
42+
role="status"
43+
aria-live="polite"
44+
aria-label="Banner announcement"
45+
>
46+
<div className={styles.text}>{text}</div>
4047
{ctaButtonText && (
4148
<Link
4249
href={ctaLink}

src/components/Navbar/Navbar.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ const Navbar = () => {
1919
const showNavbar = useDebounceNavbarVisibility(isNavbarVisible, isActive);
2020

2121
return (
22-
<div className={classNames(isBannerVisible && styles.bannerActive)}>
22+
<div className={classNames({ [styles.bannerActive]: isBannerVisible })}>
2323
<div className={styles.emptySpacePlaceholder} />
2424
<nav
2525
className={classNames(styles.container, { [styles.hiddenNav]: !showNavbar })}

src/components/Navbar/NavbarBody/index.tsx

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/* eslint-disable max-lines */
2-
import { memo, useMemo, useEffect, useRef, useState } from 'react';
2+
import { memo, useEffect, useRef, useState } from 'react';
33

44
import dynamic from 'next/dynamic';
55
import { useRouter } from 'next/router';
@@ -134,13 +134,10 @@ const NavbarBody: React.FC<Props> = ({ isBannerVisible }) => {
134134
dispatch(setDisableSearchDrawerTransition(false));
135135
};
136136

137-
const bannerProps = useMemo(
138-
() => ({
139-
text: t('stay-on-track'),
140-
ctaButtonText: t('create-my-goal'),
141-
}),
142-
[t],
143-
);
137+
const bannerProps = {
138+
text: t('stay-on-track'),
139+
ctaButtonText: t('create-my-goal'),
140+
};
144141

145142
return (
146143
<>

src/styles/theme.scss

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717
--spacing-max: 3rem;
1818

1919
--spacing-micro-px: 2px;
20-
--spacing-xxxsmall-px: 3px;
20+
--spacing-micro2-px: 3px;
2121
--spacing-xxsmall-px: 5px;
2222
--spacing-xsmall-px: 8px;
2323
--spacing-small-px: 10px;

src/styles/variables.scss

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
@use "src/styles/breakpoints";
2-
@use "src/styles/theme";
1+
@use 'src/styles/breakpoints';
2+
@use 'src/styles/theme';
33

44
// dynamic global variables
55

@@ -12,13 +12,3 @@
1212
--banner-height: 3rem;
1313
--day-circle-color: transparent;
1414
}
15-
16-
.banner-active {
17-
--banner-height: 3rem;
18-
--navbar-container-height: calc(var(--navbar-height) + var(--banner-height));
19-
20-
@include breakpoints.tablet {
21-
--banner-height: var(--navbar-height);
22-
--navbar-container-height: var(--navbar-height);
23-
}
24-
}

tests/integration/loggedin/navbar/banner.spec.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,8 @@ test.describe('Banner Test - Logged In User', () => {
1414
await page.waitForLoadState('networkidle');
1515

1616
await clickCreateMyGoalButton(page);
17-
await page.waitForURL(new RegExp(READING_GOAL_URL));
18-
expect(page.url()).toContain(READING_GOAL_URL);
17+
await page.waitForURL(new RegExp(`${READING_GOAL_URL}$`));
18+
expect(new URL(page.url()).pathname).toBe(READING_GOAL_URL);
1919
});
2020

2121
test('should redirect to /reading-goal/progress when user is logged in with goal', async ({
@@ -26,7 +26,7 @@ test.describe('Banner Test - Logged In User', () => {
2626
await page.waitForLoadState('networkidle');
2727

2828
await clickCreateMyGoalButton(page);
29-
await page.waitForURL(new RegExp(READING_GOAL_PROGRESS_URL));
30-
expect(page.url()).toContain(READING_GOAL_PROGRESS_URL);
29+
await page.waitForURL(new RegExp(`${READING_GOAL_PROGRESS_URL}$`));
30+
expect(new URL(page.url()).pathname).toBe(READING_GOAL_PROGRESS_URL);
3131
});
3232
});

0 commit comments

Comments
 (0)