-
Notifications
You must be signed in to change notification settings - Fork 48
feat(grids): Replace all toast-based feedback in grid management components #3534
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
8256403
2127f4d
3a779b8
91d34b6
0222d09
cb4d90e
e64f777
f58a335
c101221
de855f1
316fed8
bbbda65
c7fb264
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -6,7 +6,8 @@ import ReusableDialog from "@/components/shared/dialog/ReusableDialog"; | |||||||||||||||||||||||||||||||||||
| import ReusableButton from "@/components/shared/button/ReusableButton"; | ||||||||||||||||||||||||||||||||||||
| import ReusableInputField from "@/components/shared/inputfield/ReusableInputField"; | ||||||||||||||||||||||||||||||||||||
| import { Copy, Edit2, Check, X } from "lucide-react"; | ||||||||||||||||||||||||||||||||||||
| import ReusableToast from "@/components/shared/toast/ReusableToast"; | ||||||||||||||||||||||||||||||||||||
| import { useBanner } from "@/context/banner-context"; | ||||||||||||||||||||||||||||||||||||
| import { getApiErrorMessage } from "@/core/utils/getApiErrorMessage"; | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| interface AdminLevelsModalProps { | ||||||||||||||||||||||||||||||||||||
| isOpen: boolean; | ||||||||||||||||||||||||||||||||||||
|
|
@@ -15,21 +16,43 @@ interface AdminLevelsModalProps { | |||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| export function AdminLevelsModal({ isOpen, onClose }: AdminLevelsModalProps) { | ||||||||||||||||||||||||||||||||||||
| const { adminLevels, isLoading: isLoadingLevels } = useAdminLevels(); | ||||||||||||||||||||||||||||||||||||
| const { updateAdminLevel, isLoading: isUpdating } = useUpdateAdminLevel(); | ||||||||||||||||||||||||||||||||||||
| const { showBanner } = useBanner(); | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| const [editingId, setEditingId] = useState<string | null>(null); | ||||||||||||||||||||||||||||||||||||
| const [editValue, setEditValue] = useState(""); | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| const { updateAdminLevel, isLoading: isUpdating } = useUpdateAdminLevel({ | ||||||||||||||||||||||||||||||||||||
| onSuccess: (data) => { | ||||||||||||||||||||||||||||||||||||
|
Comment on lines
+24
to
+25
|
||||||||||||||||||||||||||||||||||||
| showBanner({ | ||||||||||||||||||||||||||||||||||||
| message: `Admin level updated to '${data.admin_levels.name}' successfully`, | ||||||||||||||||||||||||||||||||||||
| severity: "success", | ||||||||||||||||||||||||||||||||||||
| scoped: true, | ||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||
| setEditingId(null); | ||||||||||||||||||||||||||||||||||||
| setEditValue(""); | ||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||
|
Comment on lines
+31
to
+33
|
||||||||||||||||||||||||||||||||||||
| onError: (error) => { | ||||||||||||||||||||||||||||||||||||
| showBanner({ | ||||||||||||||||||||||||||||||||||||
| message: `Failed to update admin level: ${getApiErrorMessage(error)}`, | ||||||||||||||||||||||||||||||||||||
| severity: "error", | ||||||||||||||||||||||||||||||||||||
| scoped: true, | ||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||
|
Comment on lines
+34
to
+39
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do not render raw backend payloads in user-facing banners. At Line 33, the banner can display raw HTML/error-page bodies from the server. This leaks internal payload details and creates a broken modal UX. Proposed fix onError: (error) => {
+ const apiMessage = getApiErrorMessage(error);
+ const safeMessage = /<\/?[a-z][\s\S]*>/i.test(apiMessage)
+ ? "Server error. Please try again."
+ : apiMessage;
showBanner({
- message: `Failed to update admin level: ${getApiErrorMessage(error)}`,
+ message: `Failed to update admin level: ${safeMessage}`,
severity: "error",
scoped: true,
});
}📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. please apply coderabbit feedback @BwanikaRobert , thanks
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| const handleCopyId = async (id: string) => { | ||||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||||
| await navigator.clipboard.writeText(id); | ||||||||||||||||||||||||||||||||||||
| ReusableToast({ | ||||||||||||||||||||||||||||||||||||
| showBanner({ | ||||||||||||||||||||||||||||||||||||
|
Comment on lines
45
to
+46
|
||||||||||||||||||||||||||||||||||||
| message: "ID copied to clipboard", | ||||||||||||||||||||||||||||||||||||
| type: "SUCCESS", | ||||||||||||||||||||||||||||||||||||
| severity: "success", | ||||||||||||||||||||||||||||||||||||
| scoped: true, | ||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||
| } catch { | ||||||||||||||||||||||||||||||||||||
| ReusableToast({ | ||||||||||||||||||||||||||||||||||||
| showBanner({ | ||||||||||||||||||||||||||||||||||||
| message: "Failed to copy ID", | ||||||||||||||||||||||||||||||||||||
| type: "ERROR", | ||||||||||||||||||||||||||||||||||||
| severity: "error", | ||||||||||||||||||||||||||||||||||||
| scoped: true, | ||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||
|
|
@@ -49,15 +72,7 @@ export function AdminLevelsModal({ isOpen, onClose }: AdminLevelsModalProps) { | |||||||||||||||||||||||||||||||||||
| if (!name) { | ||||||||||||||||||||||||||||||||||||
| return; | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
| updateAdminLevel( | ||||||||||||||||||||||||||||||||||||
| { levelId, data: { name } }, | ||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||
| onSuccess: () => { | ||||||||||||||||||||||||||||||||||||
| setEditingId(null); | ||||||||||||||||||||||||||||||||||||
| setEditValue(""); | ||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||
| updateAdminLevel({ levelId, data: { name } }); | ||||||||||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use the backend-accepted update field to unblock admin-level edits. At Line 75, the mutation still sends Suggested fix- updateAdminLevel({ levelId, data: { name } });
+ updateAdminLevel({ levelId, data: { description: name } });And align the hook payload type accordingly: - useMutation<AdminLevelResponse, AxiosError<ErrorResponse>, { levelId: string; data: { name: string } }>
+ useMutation<AdminLevelResponse, AxiosError<ErrorResponse>, { levelId: string; data: { description: string } }>🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| return ( | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -5,8 +5,8 @@ import { Badge } from "@/components/ui/badge"; | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { Loader2 } from "lucide-react"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import ReusableButton from "@/components/shared/button/ReusableButton"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { AqCopy01, AqEdit01 } from "@airqo/icons-react"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import ReusableToast from "@/components/shared/toast/ReusableToast"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { Grid } from "@/app/types/grids"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { useBanner } from "@/context/banner-context"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| interface GridDetailsCardProps { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| grid: Grid; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -15,14 +15,19 @@ interface GridDetailsCardProps { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const GridDetailsCard: React.FC<GridDetailsCardProps> = ({ grid, onEdit, loading }) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const { showBanner } = useBanner(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (loading) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return <Card className="w-full rounded-lg flex flex-col justify-between items-center p-8"><Loader2 className="w-6 h-6 animate-spin" /></Card>; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const handleCopy = (text: string) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (text) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| navigator.clipboard.writeText(text); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ReusableToast({ message: "Copied to clipboard", type: "SUCCESS" }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const handleCopy = async (text: string) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (!text) return; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| await navigator.clipboard.writeText(text); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+24
to
+27
Comment on lines
+26
to
+27
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| showBanner({ message: "Copied to clipboard", severity: "success", scoped: false }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } catch { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+26
to
+29
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @Codebmk The execCommand suggestion I intentionally removed it. document.execCommand is deprecated and browsers are actively removing it. navigator.clipboard.writeText is supported on all modern browsers and the app requires HTTPS, so the Clipboard API is always available. The catch block handles failure gracefully with an error banner. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| showBanner({ message: "Failed to copy to clipboard", severity: "error", scoped: false }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+26
to
31
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+24
to
32
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Browser compatibility inconsistency: Missing fallback for older browsers. The 🔧 Proposed fix: Add browser compatibility fallback const handleCopy = async (text: string) => {
if (!text) return;
try {
- await navigator.clipboard.writeText(text);
+ if (navigator?.clipboard?.writeText) {
+ await navigator.clipboard.writeText(text);
+ } else {
+ const el = document.createElement("textarea");
+ el.value = text;
+ el.style.position = "fixed";
+ el.style.opacity = "0";
+ document.body.appendChild(el);
+ el.select();
+ document.execCommand("copy");
+ document.body.removeChild(el);
+ }
showBanner({ message: "Copied to clipboard", severity: "success", scoped: false });
} catch {
showBanner({ message: "Failed to copy to clipboard", severity: "error", scoped: false });
}
};📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Outside the scope of the PR
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
(´• ω •`) 🐇
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nice actionable feedback from coderabbit, please address this within current pr @BwanikaRobert
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Comment on lines
+24
to
32
Comment on lines
+24
to
32
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,37 +4,28 @@ import { Card } from "@/components/ui/card"; | |
| import { Loader2 } from "lucide-react"; | ||
| import ReusableButton from "@/components/shared/button/ReusableButton"; | ||
| import { AqCopy01 } from "@airqo/icons-react"; | ||
| import ReusableToast from "@/components/shared/toast/ReusableToast"; | ||
| import { Grid } from "@/app/types/grids"; | ||
| import { useBanner } from "@/context/banner-context"; | ||
|
|
||
| interface GridMeasurementsApiCardProps { | ||
| grid: Grid; | ||
| loading: boolean; | ||
| } | ||
|
|
||
| const GridMeasurementsApiCard: React.FC<GridMeasurementsApiCardProps> = ({ grid, loading }) => { | ||
| const { showBanner } = useBanner(); | ||
|
|
||
| if (loading) { | ||
| return <Card className="w-full rounded-lg flex flex-col justify-between items-center p-8"><Loader2 className="w-6 h-6 animate-spin" /></Card>; | ||
| } | ||
|
|
||
| const handleCopy = async (text: string) => { | ||
| if (!text) return; | ||
| try { | ||
| if (navigator?.clipboard?.writeText) { | ||
| await navigator.clipboard.writeText(text); | ||
| } else { | ||
| const el = document.createElement("textarea"); | ||
| el.value = text; | ||
| el.style.position = "fixed"; | ||
| el.style.opacity = "0"; | ||
| document.body.appendChild(el); | ||
| el.select(); | ||
| document.execCommand("copy"); | ||
| document.body.removeChild(el); | ||
| } | ||
| ReusableToast({ message: "API URL copied!", type: "SUCCESS" }); | ||
| await navigator.clipboard.writeText(text); | ||
|
|
||
| showBanner({ message: "API URL copied!", severity: "success", scoped: false }); | ||
|
Comment on lines
+25
to
+26
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. execCommand('copy') is deprecated. If someone is on a browser old enough to lack the Clipboard API, the rest of the app almost certainly doesn't work either.
Comment on lines
+25
to
+26
|
||
| } catch { | ||
| ReusableToast({ message: "Failed to copy to clipboard", type: "ERROR" }); | ||
| showBanner({ message: "Failed to copy to clipboard", severity: "error", scoped: false }); | ||
| } | ||
|
Comment on lines
+25
to
29
Comment on lines
+25
to
29
Comment on lines
+25
to
29
|
||
| }; | ||
|
Comment on lines
22
to
30
Comment on lines
22
to
30
Comment on lines
22
to
30
|
||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| /** | ||
| * Delay (ms) before showing a global banner after a dialog closes. | ||
| * Allows the dialog's unmount cleanup (which clears scoped banners) to complete first. | ||
| */ | ||
| export const AFTER_DIALOG_CLOSE_MS = 100; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix list indentation for markdownlint compliance.
At Line 19, list indentation is inconsistent (
MD005), which can fail docs lint checks.Suggested fix
📝 Committable suggestion
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 19-19: Inconsistent indentation for list items at the same level
Expected: 0; Actual: 1
(MD005, list-indent)
🤖 Prompt for AI Agents