From 5cab3f9e7eba11e4797dcd59757ed9e7f4b7af98 Mon Sep 17 00:00:00 2001 From: Anya Wallace Date: Mon, 2 Dec 2024 16:29:33 -0800 Subject: [PATCH 1/4] fix dropdown key bugs --- .../core/components/ComboBox/ComboBox.module.css | 15 +++++++++++++++ packages/core/components/ComboBox/index.tsx | 5 +++++ packages/core/components/DirectoryTree/index.tsx | 7 +++++-- .../EditMetadata/ExistingAnnotationPathway.tsx | 5 ++++- 4 files changed, 29 insertions(+), 3 deletions(-) diff --git a/packages/core/components/ComboBox/ComboBox.module.css b/packages/core/components/ComboBox/ComboBox.module.css index a1074965..97821031 100644 --- a/packages/core/components/ComboBox/ComboBox.module.css +++ b/packages/core/components/ComboBox/ComboBox.module.css @@ -81,6 +81,16 @@ color: unset !important; } +.combo-box-item-selected button:not(:disabled) { + background-color: var(--secondary-dark); + color: var(--highlight-text-color); +} + +.combo-box-item-checked { + background-color: var(--highlight-background-color) !important; + color: var(--highlight-text-color) !important; +} + .combo-box-item button:not(:disabled):hover, .combo-box-item > div:hover, .combo-box-item > div:hover :is(input, label) { @@ -111,6 +121,11 @@ color: var(--highlight-text-color) !important; } +.combo-box-item > button::after { + outline: unset !important; + border: unset !important; +} + .options-container { padding-bottom: var(--margin); } \ No newline at end of file diff --git a/packages/core/components/ComboBox/index.tsx b/packages/core/components/ComboBox/index.tsx index 6f6468e9..b396e954 100644 --- a/packages/core/components/ComboBox/index.tsx +++ b/packages/core/components/ComboBox/index.tsx @@ -56,6 +56,7 @@ export default function BaseComboBox(props: Props) { key={`${itemProps.key}-${itemProps.index}`} className={classNames(styles.comboBoxItem, { [styles.comboBoxItemDisabled]: !!itemProps.disabled, + [styles.comboBoxItemSelected]: itemProps.key === props.selectedKey, })} > {defaultRender(itemProps)} @@ -74,6 +75,7 @@ export default function BaseComboBox(props: Props) { disabled={props?.disabled} placeholder={placeholder} label={label} + openOnKeyboardFocus multiSelect={props?.multiSelect} options={filteredOptions} onChange={(_ev, option, _ind, value) => props.onChange?.(option, value)} @@ -88,6 +90,9 @@ export default function BaseComboBox(props: Props) { callout: styles.comboBoxCallout, optionsContainer: styles.optionsContainer, }} + comboBoxOptionStyles={{ + rootChecked: styles.comboBoxItemChecked, + }} useComboBoxAsMenuWidth={props?.useComboBoxAsMenuWidth} /> ); diff --git a/packages/core/components/DirectoryTree/index.tsx b/packages/core/components/DirectoryTree/index.tsx index 4d27a66f..59242f02 100644 --- a/packages/core/components/DirectoryTree/index.tsx +++ b/packages/core/components/DirectoryTree/index.tsx @@ -40,6 +40,7 @@ export default function DirectoryTree(props: FileListProps) { const fileService = useSelector(interaction.selectors.getFileService); const globalFilters = useSelector(selection.selectors.getFileFilters); const sortColumn = useSelector(selection.selectors.getSortColumn); + const visibleModal = useSelector(interaction.selectors.getVisibleModal); const fileSet = React.useMemo(() => { return new FileSet({ fileService: fileService, @@ -53,9 +54,11 @@ export default function DirectoryTree(props: FileListProps) { // this will effectively clear the current selection in favor of the newly navigated to row. // If at the beginning or end of a file list and attempting to navigate up or down the file selected & focused will // be in the file list above or below respectively if possible. + // Should not register key presses when an overlay modal is active React.useEffect(() => { const onArrowKeyDown = (event: KeyboardEvent) => { - if (event.code === KeyboardCode.ArrowUp) { + if (!!visibleModal) return; + else if (event.code === KeyboardCode.ArrowUp) { event.preventDefault(); // Prevent list from scrolling dispatch(selection.actions.selectNearbyFile("up", event.shiftKey)); } else if (event.code === KeyboardCode.ArrowDown) { @@ -66,7 +69,7 @@ export default function DirectoryTree(props: FileListProps) { window.addEventListener("keydown", onArrowKeyDown, true); return () => window.removeEventListener("keydown", onArrowKeyDown, true); - }, [dispatch]); + }, [dispatch, visibleModal]); const { state: { content, error, isLoading }, diff --git a/packages/core/components/EditMetadata/ExistingAnnotationPathway.tsx b/packages/core/components/EditMetadata/ExistingAnnotationPathway.tsx index 9e226a20..1c0bd2d1 100644 --- a/packages/core/components/EditMetadata/ExistingAnnotationPathway.tsx +++ b/packages/core/components/EditMetadata/ExistingAnnotationPathway.tsx @@ -11,7 +11,7 @@ import styles from "./EditMetadata.module.css"; interface ExistingAnnotationProps { onDismiss: () => void; annotationValueMap: Map | undefined; - annotationOptions: { key: string; text: string }[]; + annotationOptions: IComboBoxOption[]; selectedFileCount: number; } @@ -22,6 +22,7 @@ interface ExistingAnnotationProps { export default function ExistingAnnotationPathway(props: ExistingAnnotationProps) { const [newValues, setNewValues] = React.useState(); const [valueCount, setValueCount] = React.useState(); + const [selectedAnnotation, setSelectedAnnotation] = React.useState(); const onSelectMetadataField = ( option: IComboBoxOption | undefined, @@ -55,6 +56,7 @@ export default function ExistingAnnotationPathway(props: ExistingAnnotationProps ...valueMap, ]; } + setSelectedAnnotation(option?.key.toString()); setValueCount(valueMap); }; @@ -69,6 +71,7 @@ export default function ExistingAnnotationPathway(props: ExistingAnnotationProps className={styles.comboBox} label="Select a metadata field" placeholder="Select a field" + selectedKey={selectedAnnotation} options={props.annotationOptions} useComboBoxAsMenuWidth onChange={onSelectMetadataField} From 9f729237bb68d68dfd67b410f086f777feefe1d2 Mon Sep 17 00:00:00 2001 From: Anya Wallace Date: Fri, 6 Dec 2024 09:47:50 -0800 Subject: [PATCH 2/4] add option to prevent modal from moving --- packages/core/components/ComboBox/index.tsx | 1 + packages/core/components/Modal/BaseModal/index.tsx | 4 +++- packages/core/components/Modal/EditMetadata/index.tsx | 1 + 3 files changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/core/components/ComboBox/index.tsx b/packages/core/components/ComboBox/index.tsx index b396e954..8ef6785f 100644 --- a/packages/core/components/ComboBox/index.tsx +++ b/packages/core/components/ComboBox/index.tsx @@ -83,6 +83,7 @@ export default function BaseComboBox(props: Props) { onInputValueChange={(value) => { setSearchValue(value || ""); }} + // onPendingValueChanged={(option?: IComboBoxOption, index?: number, value?: string) => console.info("option", option, "value", value)} onRenderItem={(props, defaultRender) => onRenderItem(props, defaultRender)} styles={{ root: styles.comboBox, diff --git a/packages/core/components/Modal/BaseModal/index.tsx b/packages/core/components/Modal/BaseModal/index.tsx index 6c429786..1892add4 100644 --- a/packages/core/components/Modal/BaseModal/index.tsx +++ b/packages/core/components/Modal/BaseModal/index.tsx @@ -10,6 +10,7 @@ interface BaseModalProps { footer?: React.ReactNode; onDismiss?: () => void; title?: string; + isStatic?: boolean; // Not draggable } const DRAG_OPTIONS: IDragOptions = { @@ -31,7 +32,7 @@ export default function BaseModal(props: BaseModalProps) { isOpen onDismiss={onDismiss} containerClassName={styles.container} - dragOptions={DRAG_OPTIONS} + dragOptions={props?.isStatic ? undefined : DRAG_OPTIONS} scrollableContentClassName={styles.scrollableContainer} titleAriaId={titleId} overlay={{ className: styles.overlay }} @@ -53,4 +54,5 @@ export default function BaseModal(props: BaseModalProps) { BaseModal.defaultProps = { footer: null, onDismiss: noop, + isStatic: false, }; diff --git a/packages/core/components/Modal/EditMetadata/index.tsx b/packages/core/components/Modal/EditMetadata/index.tsx index 0ee57910..b31d105d 100644 --- a/packages/core/components/Modal/EditMetadata/index.tsx +++ b/packages/core/components/Modal/EditMetadata/index.tsx @@ -57,6 +57,7 @@ export default function EditMetadata({ onDismiss }: ModalProps) { } + isStatic onDismiss={onDismissWithWarning} title={ showWarning From 4d3624620db129a96209c7817aa7912d46d0c47d Mon Sep 17 00:00:00 2001 From: Anya Wallace Date: Fri, 6 Dec 2024 16:08:13 -0800 Subject: [PATCH 3/4] remove fuse from dropdown to prevent buggy behavior --- packages/core/components/ComboBox/index.tsx | 31 ++----------------- .../ExistingAnnotationPathway.tsx | 10 ++++-- 2 files changed, 9 insertions(+), 32 deletions(-) diff --git a/packages/core/components/ComboBox/index.tsx b/packages/core/components/ComboBox/index.tsx index 8ef6785f..4937b4f5 100644 --- a/packages/core/components/ComboBox/index.tsx +++ b/packages/core/components/ComboBox/index.tsx @@ -1,21 +1,9 @@ import { ComboBox, IComboBoxOption, IRenderFunction, ISelectableOption } from "@fluentui/react"; import classNames from "classnames"; -import Fuse from "fuse.js"; import * as React from "react"; import styles from "./ComboBox.module.css"; -const FUZZY_SEARCH_OPTIONS = { - // which keys to search on - keys: [{ name: "text", weight: 1.0 }], - - // return resulting matches sorted - shouldSort: true, - - // arbitrarily tuned; 0.0 requires a perfect match, 1.0 would match anything - threshold: 0.3, -}; - interface Props { className?: string; selectedKey?: string; @@ -34,18 +22,6 @@ interface Props { export default function BaseComboBox(props: Props) { const { options, label, placeholder } = props; - const [searchValue, setSearchValue] = React.useState(""); - - // Fuse logic borrowed from the ListPicker component - const fuse = React.useMemo(() => new Fuse(options, FUZZY_SEARCH_OPTIONS), [options]); - const filteredOptions = React.useMemo(() => { - const filteredRows = searchValue ? fuse.search(searchValue) : options; - return filteredRows.sort((a, b) => { - // If disabled, sort to the bottom - return a.disabled === b.disabled ? 0 : a.disabled ? 1 : -1; - }); - }, [options, searchValue, fuse]); - const onRenderItem = ( itemProps: ISelectableOption | undefined, defaultRender: IRenderFunction | undefined @@ -77,14 +53,11 @@ export default function BaseComboBox(props: Props) { label={label} openOnKeyboardFocus multiSelect={props?.multiSelect} - options={filteredOptions} + options={options} onChange={(_ev, option, _ind, value) => props.onChange?.(option, value)} onItemClick={(_, option) => props.onChange?.(option)} - onInputValueChange={(value) => { - setSearchValue(value || ""); - }} - // onPendingValueChanged={(option?: IComboBoxOption, index?: number, value?: string) => console.info("option", option, "value", value)} onRenderItem={(props, defaultRender) => onRenderItem(props, defaultRender)} + scrollSelectedToTop styles={{ root: styles.comboBox, label: styles.comboBoxLabel, diff --git a/packages/core/components/EditMetadata/ExistingAnnotationPathway.tsx b/packages/core/components/EditMetadata/ExistingAnnotationPathway.tsx index 1c0bd2d1..fe663851 100644 --- a/packages/core/components/EditMetadata/ExistingAnnotationPathway.tsx +++ b/packages/core/components/EditMetadata/ExistingAnnotationPathway.tsx @@ -32,7 +32,11 @@ export default function ExistingAnnotationPathway(props: ExistingAnnotationProps // FluentUI's combobox doesn't always register the entered value as an option, // so we need to be able to check both const selectedFieldName = option?.text || value; - if (!selectedFieldName) return; + if ( + !selectedFieldName || + !props.annotationOptions.some((opt) => opt.key === selectedFieldName) + ) + return; // Track how many values we've seen, since some files may not have a value for this field let totalValueCount = 0; if (props?.annotationValueMap?.has(selectedFieldName)) { @@ -56,7 +60,7 @@ export default function ExistingAnnotationPathway(props: ExistingAnnotationProps ...valueMap, ]; } - setSelectedAnnotation(option?.key.toString()); + setSelectedAnnotation(selectedFieldName); setValueCount(valueMap); }; @@ -76,7 +80,7 @@ export default function ExistingAnnotationPathway(props: ExistingAnnotationProps useComboBoxAsMenuWidth onChange={onSelectMetadataField} /> - {valueCount && ( + {!!selectedAnnotation && ( setNewValues(value)} items={valueCount || []} From 0c5c47af01b5dba0b6657865f35b4d1ce6fde902 Mon Sep 17 00:00:00 2001 From: Anya Wallace Date: Fri, 6 Dec 2024 16:36:18 -0800 Subject: [PATCH 4/4] remove unrelated modal changes from branch --- packages/core/components/Modal/BaseModal/index.tsx | 4 +--- packages/core/components/Modal/EditMetadata/index.tsx | 1 - 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/packages/core/components/Modal/BaseModal/index.tsx b/packages/core/components/Modal/BaseModal/index.tsx index 1892add4..6c429786 100644 --- a/packages/core/components/Modal/BaseModal/index.tsx +++ b/packages/core/components/Modal/BaseModal/index.tsx @@ -10,7 +10,6 @@ interface BaseModalProps { footer?: React.ReactNode; onDismiss?: () => void; title?: string; - isStatic?: boolean; // Not draggable } const DRAG_OPTIONS: IDragOptions = { @@ -32,7 +31,7 @@ export default function BaseModal(props: BaseModalProps) { isOpen onDismiss={onDismiss} containerClassName={styles.container} - dragOptions={props?.isStatic ? undefined : DRAG_OPTIONS} + dragOptions={DRAG_OPTIONS} scrollableContentClassName={styles.scrollableContainer} titleAriaId={titleId} overlay={{ className: styles.overlay }} @@ -54,5 +53,4 @@ export default function BaseModal(props: BaseModalProps) { BaseModal.defaultProps = { footer: null, onDismiss: noop, - isStatic: false, }; diff --git a/packages/core/components/Modal/EditMetadata/index.tsx b/packages/core/components/Modal/EditMetadata/index.tsx index b31d105d..0ee57910 100644 --- a/packages/core/components/Modal/EditMetadata/index.tsx +++ b/packages/core/components/Modal/EditMetadata/index.tsx @@ -57,7 +57,6 @@ export default function EditMetadata({ onDismiss }: ModalProps) { } - isStatic onDismiss={onDismissWithWarning} title={ showWarning