Skip to content

Commit 0473e7b

Browse files
BrunoQuaresmaBrunoQuaresma
committed
Apply Kayla suggestions
1 parent 0079e8e commit 0473e7b

File tree

7 files changed

+56
-68
lines changed

7 files changed

+56
-68
lines changed

‎site/src/components/Button/Button.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ export const buttonVariants = cva(
2020
default:
2121
"bg-surface-invert-primary text-content-invert hover:bg-surface-invert-secondary border-none disabled:bg-surface-secondary font-semibold",
2222
outline:
23-
"border border-border-default text-content-primary bg-transparent bg-surface-primary hover:bg-surface-secondary",
23+
"border border-border-default text-content-primary bg-transparent hover:bg-surface-secondary",
2424
subtle:
2525
"border-none bg-transparent text-content-secondary hover:text-content-primary",
2626
destructive:

‎site/src/components/ScrollArea/ScrollArea.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
1-
import * as ScrollAreaPrimitive from "@radix-ui/react-scroll-area";
21
/**
32
* Copied from shadc/ui on 03/05/2025
43
* @see {@link https://ui.shadcn.com/docs/components/scroll-area}
54
*/
5+
import * as ScrollAreaPrimitive from "@radix-ui/react-scroll-area";
66
import * as React from "react";
77
import { cn } from "utils/cn";
88

‎site/src/modules/notifications/NotificationsInbox/InboxItem.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ export const InboxItem: FC<InboxItemProps> = ({
4949

5050
<Button
5151
onClick={() => onMarkNotificationAsRead(notification.id)}
52-
className="hidden group-focus:flex group-hover:flex"
52+
className="hidden group-focus:flex group-hover:flex bg-surface-primary"
5353
variant="outline"
5454
size="sm"
5555
>

‎site/src/modules/notifications/NotificationsInbox/InboxPopover.stories.tsx

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,9 @@ import { InboxPopover } from "./InboxPopover";
66
const meta: Meta<typeof InboxPopover> = {
77
title: "modules/notifications/NotificationsInbox/InboxPopover",
88
component: InboxPopover,
9+
args: {
10+
defaultOpen: true,
11+
},
912
render: (args) => {
1013
return (
1114
<div className="w-full max-w-screen-xl p-6">
@@ -22,31 +25,28 @@ type Story = StoryObj<typeof InboxPopover>;
2225

2326
export const Default: Story = {
2427
args: {
25-
defaultOpen: true,
28+
defaultOpen: false,
2629
unreadCount: 2,
2730
notifications: MockNotifications.slice(0, 3),
2831
},
2932
};
3033

3134
export const Scrollable: Story = {
3235
args: {
33-
defaultOpen: true,
3436
unreadCount: 2,
3537
notifications: MockNotifications,
3638
},
3739
};
3840

3941
export const Loading: Story = {
4042
args: {
41-
defaultOpen: true,
4243
unreadCount: 0,
4344
notifications: undefined,
4445
},
4546
};
4647

4748
export const LoadingFailure: Story = {
4849
args: {
49-
defaultOpen: true,
5050
unreadCount: 0,
5151
notifications: undefined,
5252
error: new Error("Failed to load notifications"),
@@ -55,15 +55,13 @@ export const LoadingFailure: Story = {
5555

5656
export const Empty: Story = {
5757
args: {
58-
defaultOpen: true,
5958
unreadCount: 0,
6059
notifications: [],
6160
},
6261
};
6362

6463
export const OnRetry: Story = {
6564
args: {
66-
defaultOpen: true,
6765
unreadCount: 0,
6866
notifications: undefined,
6967
error: new Error("Failed to load notifications"),
@@ -104,7 +102,6 @@ export const OnMarkAllAsRead: Story = {
104102

105103
export const OnMarkNotificationAsRead: Story = {
106104
args: {
107-
defaultOpen: true,
108105
unreadCount: 2,
109106
notifications: MockNotifications.slice(0, 3),
110107
onMarkNotificationAsRead: fn(),

‎site/src/modules/notifications/NotificationsInbox/InboxPopover.tsx

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -77,15 +77,13 @@ export const InboxPopover: FC<InboxPopoverProps> = ({
7777
"[&>[role=menuitem]]:border-solid [&>[role=menuitem]]:border-border",
7878
])}
7979
>
80-
{notifications.map((notification) => {
81-
return (
82-
<InboxItem
83-
key={notification.id}
84-
notification={notification}
85-
onMarkNotificationAsRead={onMarkNotificationAsRead}
86-
/>
87-
);
88-
})}
80+
{notifications.map((notification) => (
81+
<InboxItem
82+
key={notification.id}
83+
notification={notification}
84+
onMarkNotificationAsRead={onMarkNotificationAsRead}
85+
/>
86+
))}
8987
</div>
9088
) : (
9189
<div className="p-6 flex items-center justify-center min-h-48">

‎site/src/modules/notifications/NotificationsInbox/NotificationsInbox.stories.tsx

Lines changed: 41 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -24,22 +24,21 @@ type Story = StoryObj<typeof NotificationsInbox>;
2424
export const Default: Story = {
2525
args: {
2626
defaultOpen: true,
27-
fetchNotifications: fn(() =>
28-
Promise.resolve({ notifications: MockNotifications, unread_count: 2 }),
29-
),
27+
fetchNotifications: fn(async () => ({
28+
notifications: MockNotifications,
29+
unread_count: 2,
30+
})),
3031
},
3132
};
3233

3334
export const Failure: Story = {
3435
args: {
3536
defaultOpen: true,
36-
fetchNotifications: fn(() =>
37-
Promise.reject(
38-
mockApiError({
39-
message: "Failed to load notifications",
40-
}),
41-
),
42-
),
37+
fetchNotifications: fn(() => {
38+
throw mockApiError({
39+
message: "Failed to load notifications",
40+
});
41+
}),
4342
},
4443
};
4544

@@ -49,23 +48,21 @@ export const FailAndRetry: Story = {
4948
fetchNotifications: (() => {
5049
let count = 0;
5150

52-
return fn(() => {
51+
return fn(async () => {
5352
count += 1;
5453

5554
// Fail on the first 3 attempts
5655
// 3 is the maximum number of retries from react-query
5756
if (count < 3) {
58-
return Promise.reject(
59-
mockApiError({
60-
message: "Failed to load notifications",
61-
}),
62-
);
57+
throw mockApiError({
58+
message: "Failed to load notifications",
59+
});
6360
}
6461

65-
return Promise.resolve({
62+
return {
6663
notifications: MockNotifications,
6764
unread_count: 2,
68-
});
65+
};
6966
});
7067
})(),
7168
},
@@ -86,10 +83,11 @@ export const FailAndRetry: Story = {
8683
export const MarkAllAsRead: Story = {
8784
args: {
8885
defaultOpen: true,
89-
fetchNotifications: fn(() =>
90-
Promise.resolve({ notifications: MockNotifications, unread_count: 2 }),
91-
),
92-
markAllAsRead: fn(() => Promise.resolve()),
86+
fetchNotifications: fn(async () => ({
87+
notifications: MockNotifications,
88+
unread_count: 2,
89+
})),
90+
markAllAsRead: fn(),
9391
},
9492
play: async ({ canvasElement }) => {
9593
const body = within(canvasElement.ownerDocument.body);
@@ -109,14 +107,15 @@ export const MarkAllAsReadFailure: Story = {
109107
decorators: [withGlobalSnackbar],
110108
args: {
111109
defaultOpen: true,
112-
fetchNotifications: fn(() =>
113-
Promise.resolve({ notifications: MockNotifications, unread_count: 2 }),
114-
),
115-
markAllAsRead: fn(() =>
116-
Promise.reject(
117-
mockApiError({ message: "Failed to mark all notifications as read" }),
118-
),
119-
),
110+
fetchNotifications: fn(async () => ({
111+
notifications: MockNotifications,
112+
unread_count: 2,
113+
})),
114+
markAllAsRead: fn(async () => {
115+
throw mockApiError({
116+
message: "Failed to mark all notifications as read",
117+
});
118+
}),
120119
},
121120
play: async ({ canvasElement }) => {
122121
const body = within(canvasElement.ownerDocument.body);
@@ -131,14 +130,11 @@ export const MarkAllAsReadFailure: Story = {
131130
export const MarkNotificationAsRead: Story = {
132131
args: {
133132
defaultOpen: true,
134-
fetchNotifications: fn(() =>
135-
Promise.resolve({ notifications: MockNotifications, unread_count: 2 }),
136-
),
137-
markNotificationAsRead: fn(() =>
138-
// true as true is necessary to solve a really strange TypeScript error
139-
// https://stackoverflow.com/questions/75864591/type-boolean-is-not-assignable-to-type-true
140-
Promise.resolve({ is_read: true as true }),
141-
),
133+
fetchNotifications: fn(async () => ({
134+
notifications: MockNotifications,
135+
unread_count: 2,
136+
})),
137+
markNotificationAsRead: fn(),
142138
},
143139
play: async ({ canvasElement }) => {
144140
const body = within(canvasElement.ownerDocument.body);
@@ -157,14 +153,13 @@ export const MarkNotificationAsReadFailure: Story = {
157153
decorators: [withGlobalSnackbar],
158154
args: {
159155
defaultOpen: true,
160-
fetchNotifications: fn(() =>
161-
Promise.resolve({ notifications: MockNotifications, unread_count: 2 }),
162-
),
163-
markNotificationAsRead: fn(() =>
164-
Promise.reject(
165-
mockApiError({ message: "Failed to mark notification as read" }),
166-
),
167-
),
156+
fetchNotifications: fn(async () => ({
157+
notifications: MockNotifications,
158+
unread_count: 2,
159+
})),
160+
markNotificationAsRead: fn(() => {
161+
throw mockApiError({ message: "Failed to mark notification as read" });
162+
}),
168163
},
169164
play: async ({ canvasElement }) => {
170165
const body = within(canvasElement.ownerDocument.body);

‎site/src/modules/notifications/NotificationsInbox/NotificationsInbox.tsx

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,7 @@ type NotificationsInboxProps = {
1616
defaultOpen?: boolean;
1717
fetchNotifications: () => Promise<NotificationsResponse>;
1818
markAllAsRead: () => Promise<void>;
19-
markNotificationAsRead: (
20-
notificationId: string,
21-
) => Promise<{ is_read: true }>;
19+
markNotificationAsRead: (notificationId: string) => Promise<void>;
2220
};
2321

2422
export const NotificationsInbox: FC<NotificationsInboxProps> = ({

0 commit comments

Comments
 (0)