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
10 changes: 10 additions & 0 deletions .Jules/palette.md
Original file line number Diff line number Diff line change
Expand Up @@ -56,3 +56,13 @@
## 2024-07-14 - Native Keyboard Submission with Forms for Modals
**Learning:** Modals designed with plain `<div>` elements as wrappers instead of `<form>` lack native keyboard submission support, forcing users to switch from keyboard to mouse to confirm actions like "Save".
**Action:** When designing modals or popups containing inputs, always use a `<form>` element to wrap the content, handle the `onSubmit` event (calling `e.preventDefault()`), and set the primary confirmation button to `type="submit"` to enable seamless Enter-key submission for keyboard users.



## 2024-07-16 - Add ARIA Labels to Generic Modal Action Buttons
**Learning:** In a canvas-based data modeling tool (like this ERD app) containing many separate modals for tasks like table editing, edge editing, and grouping, generic button text like "์ทจ์†Œ" (Cancel), "์ €์žฅ" (Save), and "์‚ญ์ œ" (Delete) causes significant ambiguity for screen reader users when isolated from visual context. When navigating forms, screen reader users might not know *which* modal's action they are triggering.
**Action:** Added explicit context to generic buttons in `EditTableModal`, `AddTableModal`, and `EditEdgeModal` using `aria-label`s (e.g., `aria-label="ํ…Œ์ด๋ธ” ํŽธ์ง‘ ์ทจ์†Œ"` or `aria-label="a์—์„œ b ๊ด€๊ณ„ ์‚ญ์ œ"`). Always remember to update the corresponding tests (e.g. `getByRole`) whenever `aria-label` is changed.

## 2026-07-16 - Safe Component Unmounting in React Modals
**Learning:** When a modal is closed, unsubmitted form state can inadvertently persist in parent component structures if state reset logic is absent. In `EditTableModal.tsx`, editing the graph (e.g. adding or removing columns) mutates `setNodes` directly, but clicking 'Cancel' only dismissed the modal. This creates a state desync where canceled changes remain in the diagram. Strix security scanners flag this as a business-logic integrity flaw.
**Action:** For modals that perform optimistic state updates on shared state objects (like nodes in a canvas), always take a deep-copy snapshot of the initial state upon opening. Ensure `onCancel` handles reverting the parent state back to this snapshot before dismissal.
3 changes: 2 additions & 1 deletion frontend/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -36,5 +36,6 @@
},
"overrides": {
"esbuild": "^0.25.0"
}
},
"packageManager": "pnpm@10.30.3"
}
3 changes: 2 additions & 1 deletion frontend/src/components/modals/AddTableModal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -75,10 +75,11 @@ export function AddTableModal({
className="row"
style={{ justifyContent: "flex-end", marginTop: 8 }}
>
<button type="button" onClick={onAddTableCancel}>์ทจ์†Œ</button>
<button type="button" onClick={onAddTableCancel} aria-label="ํ…Œ์ด๋ธ” ์ถ”๊ฐ€ ์ทจ์†Œ">์ทจ์†Œ</button>
<button
type="submit"
disabled={!newTableName.trim()}
aria-label="ํ…Œ์ด๋ธ” ์ถ”๊ฐ€ ์ €์žฅ"
style={
newTableName.trim()
? { background: "#034ea2", color: "#fff" }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ describe('modal dialog accessibility', () => {
);

const tableNameInput = screen.getByLabelText('ํ…Œ์ด๋ธ” ์ด๋ฆ„');
const saveButton = screen.getByRole('button', { name: '์ €์žฅ' });
const saveButton = screen.getByRole('button', { name: 'ํ…Œ์ด๋ธ” ์ถ”๊ฐ€ ์ €์žฅ' });

await waitFor(() => expect(tableNameInput).toHaveFocus());

Expand Down
4 changes: 3 additions & 1 deletion frontend/src/components/modals/EditEdgeModal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -83,14 +83,16 @@ export function EditEdgeModal({
type="button"
onClick={onRelDelete}
style={{ color: "#b91c1c", borderColor: "#fca5a5" }}
aria-label={`${editingEdge.source}์—์„œ ${editingEdge.target} ๊ด€๊ณ„ ์‚ญ์ œ`}
>
์‚ญ์ œ
</button>
<div className="row">
<button type="button" onClick={onRelCancel}>์ทจ์†Œ</button>
<button type="button" onClick={onRelCancel} aria-label="๊ด€๊ณ„ ์„ค์ • ์ทจ์†Œ">์ทจ์†Œ</button>
<button
type="submit"
style={{ background: "#034ea2", color: "#fff" }}
aria-label="๊ด€๊ณ„ ์„ค์ • ์ €์žฅ"
>
์ €์žฅ
</button>
Expand Down
49 changes: 33 additions & 16 deletions frontend/src/components/modals/EditTableModal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,27 @@ export function EditTableModal({
onEditTableSubmit,
onDeleteTable,
}: EditTableModalProps) {
const dialogRef = useDialogAccessibility(isOpen && Boolean(editingNode), onEditTableCancel);
// Snapshot the original node so we can restore it on cancel if needed
const [originalNodeSnapshot, setOriginalNodeSnapshot] = React.useState<Node<TableNodeData> | null>(null);

React.useEffect(() => {
if (isOpen && editingNode && !originalNodeSnapshot) {
setOriginalNodeSnapshot(JSON.parse(JSON.stringify(editingNode)));
}
if (!isOpen) {
setOriginalNodeSnapshot(null);
}
}, [isOpen, editingNode, originalNodeSnapshot]);

const handleCancel = () => {
if (originalNodeSnapshot) {
setNodes((nds) => nds.map((n) => n.id === originalNodeSnapshot.id ? originalNodeSnapshot : n));
setEditingNode(originalNodeSnapshot);
}
onEditTableCancel();
};

const dialogRef = useDialogAccessibility(isOpen && Boolean(editingNode), handleCancel);

if (!isOpen || !editingNode) return null;

Expand All @@ -31,7 +51,7 @@ export function EditTableModal({
<div className="modal" style={{ width: 800, maxWidth: "90vw", maxHeight: "90vh", display: "flex", flexDirection: "column" }} role="dialog" aria-modal="true" aria-labelledby="edit-table-title" ref={dialogRef} tabIndex={-1}>
<div className="modal__header">
<h3 id="edit-table-title">ํ…Œ์ด๋ธ” ํŽธ์ง‘</h3>
<button type="button" aria-label="๋‹ซ๊ธฐ" onClick={onEditTableCancel}>X</button>
<button type="button" aria-label="๋‹ซ๊ธฐ" onClick={handleCancel}>X</button>
</div>
<div style={{ overflowY: "auto", padding: "0 4px", flex: 1 }}>
<form id="editTableForm" onSubmit={onEditTableSubmit} className="col" style={{ gap: 12 }}>
Expand Down Expand Up @@ -61,6 +81,12 @@ export function EditTableModal({
<button
type="button"
onClick={() => {
const newCol = {
column_name: `new_col_${Date.now()}`,
data_type: "text",
is_not_null: false,
is_pk: false,
};
setNodes((nds: Node<TableNodeData>[]) =>
nds.map((n: Node<TableNodeData>) => {
if (n.id === editingNode.id) {
Expand All @@ -70,12 +96,7 @@ export function EditTableModal({
...n.data,
columns: [
...n.data.columns,
{
column_name: `new_col_${Date.now()}`,
data_type: "text",
is_not_null: false,
is_pk: false,
}
{ ...newCol }
]
}
};
Expand All @@ -91,12 +112,7 @@ export function EditTableModal({
...prev.data,
columns: [
...prev.data.columns,
{
column_name: `new_col_${Date.now()}`,
data_type: "text",
is_not_null: false,
is_pk: false,
}
{ ...newCol }
]
}
}
Expand Down Expand Up @@ -215,19 +231,20 @@ export function EditTableModal({
},
},
]);
onEditTableCancel();
handleCancel();
}}
style={{ color: "#034ea2", borderColor: "#93c5fd" }}
>
๋ณต์ œ
</button>
</div>
<div className="row">
<button type="button" onClick={onEditTableCancel}>์ทจ์†Œ</button>
<button type="button" onClick={handleCancel} aria-label="ํ…Œ์ด๋ธ” ํŽธ์ง‘ ์ทจ์†Œ">์ทจ์†Œ</button>
<button
type="submit"
form="editTableForm"
style={{ background: "#034ea2", color: "#fff" }}
aria-label="ํ…Œ์ด๋ธ” ํŽธ์ง‘ ์ €์žฅ"
>
์ €์žฅ
</button>
Expand Down
12 changes: 6 additions & 6 deletions frontend/src/components/modals/ModalCoverage.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ describe('modal behavior coverage', () => {
fireEvent.submit(screen.getByRole('dialog'))
expect(setNewTableName).toHaveBeenCalledWith('users')
expect(onSubmit).not.toHaveBeenCalled()
fireEvent.click(screen.getByRole('button', { name: '์ทจ์†Œ' }))
fireEvent.click(screen.getByRole('button', { name: 'ํ…Œ์ด๋ธ” ์ถ”๊ฐ€ ์ทจ์†Œ' }))
expect(onCancel).toHaveBeenCalledOnce()

rerender(
Expand All @@ -83,7 +83,7 @@ describe('modal behavior coverage', () => {
)
fireEvent.submit(screen.getByRole('dialog'))
expect(onSubmit).toHaveBeenCalledOnce()
expect(screen.getByRole('button', { name: '์ €์žฅ' })).toBeEnabled()
expect(screen.getByRole('button', { name: 'ํ…Œ์ด๋ธ” ์ถ”๊ฐ€ ์ €์žฅ' })).toBeEnabled()
})

it('covers EditEdgeModal visibility and actions', () => {
Expand Down Expand Up @@ -116,9 +116,9 @@ describe('modal behavior coverage', () => {
fireEvent.change(screen.getByLabelText('์ œ์•ฝ์กฐ๊ฑด ์ด๋ฆ„ (Label)'), {
target: { value: 'fk_changed' },
})
fireEvent.click(screen.getByRole('button', { name: '์‚ญ์ œ' }))
fireEvent.click(screen.getByRole('button', { name: '์ทจ์†Œ' }))
fireEvent.click(screen.getByRole('button', { name: '์ €์žฅ' }))
fireEvent.click(screen.getByRole('button', { name: 'a์—์„œ b ๊ด€๊ณ„ ์‚ญ์ œ' }))
fireEvent.click(screen.getByRole('button', { name: '๊ด€๊ณ„ ์„ค์ • ์ทจ์†Œ' }))
fireEvent.click(screen.getByRole('button', { name: '๊ด€๊ณ„ ์„ค์ • ์ €์žฅ' }))
expect(setRelLabel).toHaveBeenCalledWith('fk_changed')
expect(onDelete).toHaveBeenCalledOnce()
expect(onCancel).toHaveBeenCalledOnce()
Expand Down Expand Up @@ -191,7 +191,7 @@ describe('modal behavior coverage', () => {
data: { title: 'public.users_copy' },
})
expect(duplicated.data.columns).not.toBe(tableNode.data.columns)
fireEvent.click(screen.getByRole('button', { name: '์ทจ์†Œ' }))
fireEvent.click(screen.getByRole('button', { name: 'ํ…Œ์ด๋ธ” ํŽธ์ง‘ ์ทจ์†Œ' }))
fireEvent.click(screen.getByRole('button', { name: '๋‹ซ๊ธฐ' }))
expect(onSubmit).toHaveBeenCalledOnce()
expect(onDeleteTable).toHaveBeenCalledOnce()
Expand Down
Loading