Skip to content

Commit e1a4e21

Browse files
Use <dialog> for modals, guard on falsy update return, and relax gloss/senseRef constraint
- Replace `<div>` containers with `<dialog open aria-labelledby="…">` in `CreateProjectModal`, `ProjectMetadataModal`, and `SelectInterlinearProjectModal` for proper accessibility semantics. - In `ProjectMetadataModal.handleSave`, return early when `updateProjectMetadata` resolves with a falsy value (mirrors the existing guard in `CreateProjectModal.handleCreate`). - In `interlinearizer.d.ts`, replace the discriminated union on `Token` and `Phrase` that forbade setting both `gloss`/`glossSenseRef` (or `gloss`/`senseRef`) with optional fields, reflecting the updated rule that `gloss` serves as a local override when both are present. - Add a test for the falsy-return early-exit path in `ProjectMetadataModal`, and add the missing `waitFor` before negative assertions in the `CreateProjectModal` falsy-return test.
1 parent 2c31e70 commit e1a4e21

6 files changed

Lines changed: 72 additions & 30 deletions

File tree

src/__tests__/components/CreateProjectModal.test.tsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,7 @@ describe('CreateProjectModal', () => {
159159

160160
await userEvent.click(screen.getByRole('button', { name: /^create$/i }));
161161

162+
await waitFor(() => expect(papi.commands.sendCommand).toHaveBeenCalled());
162163
expect(onProjectCreated).not.toHaveBeenCalled();
163164
expect(onClose).not.toHaveBeenCalled();
164165
});

src/__tests__/components/ProjectMetadataModal.test.tsx

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ const testProps = {
4343
describe('ProjectMetadataModal', () => {
4444
beforeEach(() => {
4545
jest.mocked(useLocalizedStrings).mockReturnValue([LOCALIZED, false]);
46-
mockSendCommand.mockResolvedValue(undefined);
46+
mockSendCommand.mockResolvedValue('{}');
4747
jest.mocked(papi.notifications.send).mockResolvedValue('mock-notification-id');
4848
});
4949

@@ -193,6 +193,21 @@ describe('ProjectMetadataModal', () => {
193193
);
194194
});
195195

196+
it('does not call onProjectSaved or onClose when save sendCommand resolves with a falsy value', async () => {
197+
mockSendCommand.mockResolvedValue(undefined);
198+
const onProjectSaved = jest.fn();
199+
const onClose = jest.fn();
200+
render(
201+
<ProjectMetadataModal {...testProps} onProjectSaved={onProjectSaved} onClose={onClose} />,
202+
);
203+
204+
await userEvent.click(screen.getByRole('button', { name: /^save$/i }));
205+
206+
await waitFor(() => expect(mockSendCommand).toHaveBeenCalled());
207+
expect(onProjectSaved).not.toHaveBeenCalled();
208+
expect(onClose).not.toHaveBeenCalled();
209+
});
210+
196211
it('does not call onProjectSaved or onClose when save sendCommand rejects, but sends an error notification', async () => {
197212
mockSendCommand.mockRejectedValue(new Error('save failed'));
198213
const onProjectSaved = jest.fn();

src/components/CreateProjectModal.tsx

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -82,8 +82,12 @@ export function CreateProjectModal({
8282

8383
return (
8484
<div className="tw-fixed tw-inset-0 tw-z-50 tw-flex tw-items-center tw-justify-center tw-bg-black/40">
85-
<div className="tw-bg-background tw-rounded tw-border tw-border-border tw-p-6 tw-w-96 tw-shadow-lg">
86-
<h2 className="tw-text-base tw-font-semibold tw-mb-4">
85+
<dialog
86+
aria-labelledby="create-project-modal-title"
87+
className="tw-bg-background tw-rounded tw-border tw-border-border tw-p-6 tw-w-96 tw-shadow-lg"
88+
open
89+
>
90+
<h2 id="create-project-modal-title" className="tw-text-base tw-font-semibold tw-mb-4">
8791
{localizedStrings['%interlinearizer_modal_create_title%']}
8892
</h2>
8993
<label className="tw-block tw-text-sm tw-mb-1" htmlFor="project-name">
@@ -125,7 +129,7 @@ export function CreateProjectModal({
125129
{localizedStrings['%interlinearizer_modal_create_submit%']}
126130
</Button>
127131
</div>
128-
</div>
132+
</dialog>
129133
</div>
130134
);
131135
}

src/components/ProjectMetadataModal.tsx

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -92,13 +92,14 @@ export function ProjectMetadataModal({
9292
const newDescription = editDescription || undefined;
9393
const newLanguage = editLanguage.trim();
9494
try {
95-
await papi.commands.sendCommand(
95+
const updatedProjectJson = await papi.commands.sendCommand(
9696
'interlinearizer.updateProjectMetadata',
9797
interlinearProjectId,
9898
newName,
9999
newDescription,
100100
newLanguage,
101101
);
102+
if (!updatedProjectJson) return;
102103
onProjectSaved?.({
103104
name: newName,
104105
description: newDescription,
@@ -136,8 +137,12 @@ export function ProjectMetadataModal({
136137

137138
return (
138139
<div className="tw-fixed tw-inset-0 tw-z-50 tw-flex tw-items-center tw-justify-center tw-bg-black/40">
139-
<div className="tw-bg-background tw-rounded-lg tw-border tw-border-border tw-p-6 tw-w-[32rem] tw-shadow-lg">
140-
<h2 className="tw-text-base tw-font-semibold tw-mb-4">
140+
<dialog
141+
aria-labelledby="project-metadata-modal-title"
142+
className="tw-bg-background tw-rounded-lg tw-border tw-border-border tw-p-6 tw-w-[32rem] tw-shadow-lg"
143+
open
144+
>
145+
<h2 id="project-metadata-modal-title" className="tw-text-base tw-font-semibold tw-mb-4">
141146
{localizedStrings['%interlinearizer_modal_metadata_title%']}
142147
</h2>
143148

@@ -249,7 +254,7 @@ export function ProjectMetadataModal({
249254
</div>
250255
</div>
251256
)}
252-
</div>
257+
</dialog>
253258
</div>
254259
);
255260
}

src/components/SelectInterlinearProjectModal.tsx

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -127,8 +127,12 @@ export function SelectInterlinearProjectModal({
127127

128128
return (
129129
<div className="tw-fixed tw-inset-0 tw-z-50 tw-flex tw-items-center tw-justify-center tw-bg-black/40">
130-
<div className="tw-bg-background tw-rounded-lg tw-border tw-border-border tw-p-6 tw-w-[32rem] tw-shadow-lg">
131-
<h2 className="tw-text-base tw-font-semibold tw-mb-4">
130+
<dialog
131+
aria-labelledby="select-project-modal-title"
132+
className="tw-bg-background tw-rounded-lg tw-border tw-border-border tw-p-6 tw-w-[32rem] tw-shadow-lg"
133+
open
134+
>
135+
<h2 id="select-project-modal-title" className="tw-text-base tw-font-semibold tw-mb-4">
132136
{localizedStrings['%interlinearizer_modal_select_title%']}
133137
</h2>
134138

@@ -175,7 +179,7 @@ export function SelectInterlinearProjectModal({
175179
{localizedStrings['%interlinearizer_modal_select_create_new%']}
176180
</Button>
177181
</div>
178-
</div>
182+
</dialog>
179183
</div>
180184
);
181185
}

src/types/interlinearizer.d.ts

Lines changed: 32 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -640,7 +640,8 @@ declare module 'interlinearizer' {
640640
* `gloss` is a free-form gloss string for the token (keyed by analysis-language tag).
641641
* `glossSenseRef` alternatively resolves the gloss through a specific `ISense` in the Lexicon
642642
* extension — when set, the rendered gloss is the sense's gloss text and may be refreshed
643-
* automatically if the lexicon is edited. Setting both is a type error: use one or the other.
643+
* automatically if the lexicon is edited. Both may be set concurrently: when both are present,
644+
* `gloss` serves as a local override that takes precedence for rendering.
644645
*
645646
* `morphemes` carries the parse information. Each morpheme links to the Lexicon extension via
646647
* `entryRef` / `senseRef`.
@@ -719,16 +720,21 @@ declare module 'interlinearizer' {
719720
* becomes a concern (token text is typically short, so the literal string is usually fine).
720721
*/
721722
tokenSnapshot?: string;
722-
} &
723+
723724
/**
724-
* Exactly one of `gloss` or `glossSenseRef` may be set, or neither — but never both. Use
725-
* `gloss` for a free-form string; use `glossSenseRef` to derive the rendered gloss from a
726-
* Lexicon sense (enabling automatic refresh when the lexicon is edited).
725+
* Free-form gloss string for this token, keyed by analysis-language BCP 47 tag. May coexist
726+
* with `glossSenseRef` when the user has both a local override and a lexicon-backed sense — the
727+
* local gloss takes precedence for rendering in that case.
727728
*/
728-
(| { gloss: MultiString; glossSenseRef?: never }
729-
| { glossSenseRef: SenseRef; gloss?: never }
730-
| { gloss?: never; glossSenseRef?: never }
731-
);
729+
gloss?: MultiString;
730+
731+
/**
732+
* Resolves the gloss through a specific `ISense` in the Lexicon extension, enabling automatic
733+
* refresh when the lexicon is edited. May coexist with `gloss` when the user maintains both a
734+
* lexicon-linked sense and a local override.
735+
*/
736+
glossSenseRef?: SenseRef;
737+
};
732738

733739
/**
734740
* An ordered morpheme within a token's parse.
@@ -811,8 +817,9 @@ declare module 'interlinearizer' {
811817
*
812818
* `gloss` is a free-form phrase gloss. `senseRef` alternatively points at a lexicon sense when
813819
* the phrase is a multi-word lexical entry — the Lexicon extension supports both kinds via
814-
* `IEntry.morphType = Phrase` (contiguous) or `DiscontiguousPhrase` (e.g. "ne … pas"). Setting
815-
* both is a type error: use one or the other.
820+
* `IEntry.morphType = Phrase` (contiguous) or `DiscontiguousPhrase` (e.g. "ne … pas"). Both may
821+
* be set concurrently: when both are present, `gloss` serves as a local override that takes
822+
* precedence for rendering.
816823
*
817824
* Provenance fields (`producer`, `sourceUser`, `confidence`, `status`) let a suggestion engine
818825
* record proposed phrases that a user can then approve or reject, enabling automated recognition
@@ -866,16 +873,22 @@ declare module 'interlinearizer' {
866873
* maintain this alignment when filtering or transforming tokens.
867874
*/
868875
tokenSnapshots?: [string, ...string[]];
869-
} &
876+
870877
/**
871-
* Exactly one of `gloss` or `senseRef` may be set, or neither — but never both. Use `gloss` for
872-
* a free-form phrase gloss; use `senseRef` when the phrase is a multi-word lexical entry in the
873-
* Lexicon extension (enabling automatic gloss refresh).
878+
* Free-form phrase gloss keyed by analysis-language BCP 47 tag. May coexist with `senseRef`
879+
* when the user has both a local override and a lexicon-backed sense — the local gloss takes
880+
* precedence for rendering in that case.
874881
*/
875-
(| { gloss: MultiString; senseRef?: never }
876-
| { senseRef: SenseRef; gloss?: never }
877-
| { gloss?: never; senseRef?: never }
878-
);
882+
gloss?: MultiString;
883+
884+
/**
885+
* Points at a multi-word lexical entry in the Lexicon extension (e.g. `IEntry.morphType =
886+
* Phrase` or `DiscontiguousPhrase`), enabling automatic gloss refresh when the lexicon is
887+
* edited. May coexist with `gloss` when the user maintains both a lexicon-linked sense and a
888+
* local override.
889+
*/
890+
senseRef?: SenseRef;
891+
};
879892

880893
// ---------------------------------------------------------------------------
881894
// §7 AlignmentLink, AlignmentEndpoint

0 commit comments

Comments
 (0)