Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 22 additions & 1 deletion apps/site/components/withBanner.tsx
Original file line number Diff line number Diff line change
@@ -1,24 +1,45 @@
'use client';

import { ArrowUpRightIcon } from '@heroicons/react/24/outline';
import Banner from '@node-core/ui-components/Common/Banner';
import { useTranslations } from 'next-intl';
import { useEffect, useState } from 'react';

import Link from '#site/components/Link';
import { siteConfig } from '#site/next.json.mjs';
import { dateIsBetween } from '#site/util/date';

import type { FC } from 'react';

const STORAGE_KEY = 'banner-dismissal';

const WithBanner: FC<{ section: string }> = ({ section }) => {
const banner = siteConfig.websiteBanners[section];
const t = useTranslations();

if (banner && dateIsBetween(banner.startDate, banner.endDate)) {
const [dismissed, setDismissed] = useState(false);

// localStorage is only available on the client, so the banner is always
// rendered on the server and hidden after hydration if previously dismissed
useEffect(() => {
if (banner) {
setDismissed(localStorage.getItem(STORAGE_KEY) === banner.text);

Check warning on line 26 in apps/site/components/withBanner.tsx

View workflow job for this annotation

GitHub Actions / Quality checks

Do not call the 'set' function 'setDismissed' of 'useState' synchronously in an effect. This can lead to unnecessary re-renders and performance issues

Check warning on line 26 in apps/site/components/withBanner.tsx

View workflow job for this annotation

GitHub Actions / Quality checks

Do not call the 'set' function 'setDismissed' of 'useState' synchronously in an effect. This can lead to unnecessary re-renders and performance issues
}
}, [banner]);
Comment thread
cursor[bot] marked this conversation as resolved.

if (banner && !dismissed && dateIsBetween(banner.startDate, banner.endDate)) {
const bannerType = banner.type || 'default';

const onClose = () => {
localStorage.setItem(STORAGE_KEY, banner.text);
setDismissed(true);
};

return (
<Banner
type={banner.type}
aria-label={t(`components.banner.${bannerType}`)}
onClose={onClose}
>
{banner.link ? (
<Link href={banner.link}>{banner.text}</Link>
Expand Down
2 changes: 1 addition & 1 deletion packages/ui-components/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@node-core/ui-components",
"version": "1.7.1",
"version": "1.7.2",
"type": "module",
"exports": {
"./*": {
Expand Down
15 changes: 14 additions & 1 deletion packages/ui-components/src/Common/Banner/index.module.css
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
@reference "../../styles/index.css";

.banner {
@apply flex
@apply relative
flex
w-full
flex-row
items-center
Expand Down Expand Up @@ -40,3 +41,15 @@
.warning {
@apply bg-warning-600;
}

.closeButton {
@apply absolute
right-4
text-lg;

span {
@apply text-white/60
transition-colors
hover:text-white;
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Close button overlaps content

Medium Severity

The close control is absolute with right-4 while the banner still uses symmetric px-8 and centered content. Nothing reserves space for the button, so on narrower viewports or with longer banner text the label, link, or icon can sit under the ×. Elsewhere in this package, absolute controls get matching padding so content stays clear.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit bd140ea. Configure here.

8 changes: 8 additions & 0 deletions packages/ui-components/src/Common/Banner/index.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -26,4 +26,12 @@ export const Warning: Story = {
},
};

export const Dismissible: Story = {
args: {
children: 'Lorem ipsum dolor sit amet, consectetur adipiscing elit.',
type: 'default',
onClose: () => {},
},
};

export default { component: Banner } as Meta;
23 changes: 21 additions & 2 deletions packages/ui-components/src/Common/Banner/index.tsx
Original file line number Diff line number Diff line change
@@ -1,21 +1,40 @@
import type { FC, HTMLAttributes, PropsWithChildren } from 'react';
import classNames from 'classnames';

import type {
FC,
HTMLAttributes,
MouseEventHandler,
PropsWithChildren,
} from 'react';

import styles from './index.module.css';

export type BannerProps = {
type?: 'default' | 'warning' | 'error';
onClose?: MouseEventHandler<HTMLButtonElement>;
} & HTMLAttributes<HTMLElement>;

const Banner: FC<PropsWithChildren<BannerProps>> = ({
type = 'default',
onClose,
children,
...props
}) => (
<section
className={`${styles.banner} ${styles[type] || styles.default}`}
className={classNames(styles.banner, styles[type] || styles.default)}
{...props}
>
{children}
{onClose && (
<button
type="button"
className={styles.closeButton}
aria-label="Close banner"
onClick={onClose}
>
<span aria-hidden="true">&times;</span>
</button>
Comment thread
MattIPv4 marked this conversation as resolved.
Comment thread
cursor[bot] marked this conversation as resolved.
)}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Close label not localizable

Low Severity

The close control hardcodes aria-label="Close banner" on the button, and that string is not overridable via props. Extra HTML attributes only spread onto the section, so consumers such as withBanner cannot pass a translated label despite already using useTranslations for the banner itself.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit b2dc64f. Configure here.

</section>
);

Expand Down
Loading