Skip to content
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

[DataEntryTable] Fix NewEntry vernacular focus issues #3297

Merged
merged 3 commits into from
Aug 8, 2024
Merged
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
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ interface DeleteEntryProps {
// and deletion will happen when the button is pressed
confirmId?: string;
disabled?: boolean;
wordId?: string;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@ export default function GlossWithSuggestions(
filterOptions={(options) => options}
// freeSolo allows use of a typed entry not available as a drop-down option
freeSolo
includeInputInList
// option-never-equals-value prevents automatic option highlighting
isOptionEqualToValue={() => false}
options={spellChecker.getSpellingSuggestions(props.gloss)}
Expand All @@ -58,13 +57,7 @@ export default function GlossWithSuggestions(
props.onBlur();
}
}}
onChange={(_e, newValue) => {
// onChange is triggered when an option is selected
props.updateGlossField(newValue ?? "");
}}
inputValue={props.gloss}
onInputChange={(_e, newInputValue) => {
// onInputChange is triggered by typing
props.updateGlossField(newInputValue);
}}
renderInput={(params) => (
Expand All @@ -83,11 +76,17 @@ export default function GlossWithSuggestions(
{...liProps}
analysis
aria-selected={selected}
key={option}
lang={props.analysisLang.bcp47}
>
{SpellChecker.replaceAllButLastWordWithEllipses(option)}
</LiWithFont>
)}
/* Even though `onKeyPress` is deprecated, we need to keep using it:
* - `onKeyDown` doesn't work with spelling suggestion selection via Enter,
* because the submission occurs before the selected suggestion is applied;
* - `onKeyUp` doesn't work with SenseDialog selection via Enter,
* because the dialog closes before the key is released. */
onKeyPress={(e: KeyboardEvent) => {
if (e.key === Key.Enter) {
props.handleEnter();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,14 +47,11 @@ export default function VernWithSuggestions(
freeSolo
value={props.vernacular}
options={props.suggestedVerns ?? []}
// option-never-equals-value prevents automatic option highlighting
isOptionEqualToValue={() => false}
onBlur={props.onBlur}
onChange={(_e, value) => {
// onChange is triggered when an option is selected
props.updateVernField(value ?? "", true);
}}
onFocus={props.onFocus}
onInputChange={(_e, value) => {
// onInputChange is triggered by typing
props.updateVernField(value);
}}
onKeyPress={(e: KeyboardEvent) => {
Expand All @@ -74,7 +71,12 @@ export default function VernWithSuggestions(
/>
)}
renderOption={(liProps, option, { selected }) => (
<LiWithFont {...liProps} aria-selected={selected} vernacular>
<LiWithFont
{...liProps}
aria-selected={selected}
key={option}
vernacular
>
{option}
</LiWithFont>
)}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ export default function VernDialog(props: vernDialogProps): ReactElement {

return (
<Dialog
disableRestoreFocus
maxWidth={false}
onClose={(_, reason) => {
if (reason !== "backdropClick") {
Expand Down
24 changes: 8 additions & 16 deletions src/components/DataEntry/DataEntryTable/NewEntry/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -206,19 +206,12 @@ export default function NewEntry(props: NewEntryProps): ReactElement {
}
};

const handleEnter = async (checkGloss: boolean): Promise<void> => {
const handleGlossEnter = async (): Promise<void> => {
// The user can never submit a new entry without a vernacular
if (newVern) {
// The user can conditionally submit a new entry without a gloss
if (newGloss || !checkGloss) {
await addOrUpdateWord();
focus(FocusTarget.Vernacular);
} else {
focus(FocusTarget.Gloss);
}
} else {
focus(FocusTarget.Vernacular);
await addOrUpdateWord();
}
focus(FocusTarget.Vernacular);
};

/** Clear the duplicate selection if user returns to the vernacular field. */
Expand Down Expand Up @@ -269,9 +262,10 @@ export default function NewEntry(props: NewEntryProps): ReactElement {
}}
onFocus={handleOnVernFocus}
suggestedVerns={suggestedVerns}
// To prevent unintentional no-gloss submissions:
// If enter pressed from the vern field, check whether gloss is empty
handleEnter={() => handleEnter(true)}
// To prevent unintentional no-gloss or wrong-gloss submissions
// and to simplify interactions with Autocomplete and with the dialogs:
// if Enter is pressed from the vern field, move focus to gloss field.
handleEnter={() => focus(FocusTarget.Gloss)}
vernacularLang={vernacularLang}
textFieldId={NewEntryId.TextFieldVern}
onUpdate={() => conditionalFocus(FocusTarget.Vernacular)}
Expand All @@ -298,9 +292,7 @@ export default function NewEntry(props: NewEntryProps): ReactElement {
gloss={newGloss}
glossInput={glossInput}
updateGlossField={setNewGloss}
// To allow intentional no-gloss submissions:
// If enter pressed from the gloss field, don't check whether gloss is empty
handleEnter={() => handleEnter(false)}
handleEnter={() => handleGlossEnter()}
analysisLang={analysisLang}
textFieldId={NewEntryId.TextFieldGloss}
onUpdate={() => conditionalFocus(FocusTarget.Gloss)}
Expand Down
1 change: 0 additions & 1 deletion src/components/DataEntry/DataEntryTable/RecentEntry.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,6 @@ export function RecentEntry(props: RecentEntryProps): ReactElement {
buttonId={`${idAffix}-${props.rowIndex}-delete`}
confirmId={"addWords.deleteRowWarning"}
disabled={editing || props.disabled}
wordId={props.entry.id}
/>
</Grid>
</Grid>
Expand Down
Loading