Skip to content

chore(combobox): remove combobox clone COMPASS-6465 #6562

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.

Sign up for GitHub

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

Merged
merged 7 commits into from
Dec 12, 2024
Merged

chore(combobox): remove combobox clone COMPASS-6465 #6562

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
24503af
remove combobox clone
mabaasit Dec 10, 2024
6518809
fix positioning
mabaasit Dec 11, 2024
878d34c
fix selector
mabaasit Dec 11, 2024
cad8696
clean up css
mabaasit Dec 12, 2024
efe3b8e
fix types
mabaasit Dec 12, 2024
be538fb
drive-by
mabaasit Dec 12, 2024
d903db6
Merge branch 'main' into remove-combobox
mabaasit Dec 12, 2024
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
288 changes: 247 additions & 41 deletions package-lock.json
View file
Open in desktop

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion ...src/components/pipeline-builder-workspace/pipeline-builder-ui-workspace/sortable-list.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ const SortableItem = ({ id, index, type }: SortableItemProps) => {
const style = {
transform: cssDndKit.Transform.toString(transform),
transition,
zIndex: isDragging ? 1 : 0,
...(isDragging ? { zIndex: 1 } : {}),
};

const sortableProps: SortableProps = {
Expand Down
76 changes: 46 additions & 30 deletions packages/compass-aggregations/src/components/stage-toolbar/stage-operator-select.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,21 +17,34 @@ import { filterStageOperators } from '../../utils/stage';
import { isAtlasOnly } from '../../utils/stage';
import type { ServerEnvironment } from '../../modules/env';

const inputWidth = spacing[7] * 2;
const inputWidth = spacing[1400] * 3;
const inputHeight = spacing[600] - 2; // match other xs controls
// width of options popover
const comboxboxOptionsWidth = spacing[1200] * 10;
// left position of options popover wrt input. this aligns it with the start of input
const comboboxOptionsLeft = (comboxboxOptionsWidth - inputWidth) / 2;

const inputHeight = spacing[4] - 2; // match other xs controls
const comboboxStyles = css({
width: inputWidth,
'& [role="combobox"]': {
padding: 0,
paddingLeft: spacing[1],
paddingLeft: spacing[100],
height: inputHeight,
'& > div': {
minHeight: inputHeight,
},
'& input': {
height: inputHeight - 2,
},
},
});

const comboboxPortalStyles = css({
position: 'fixed',
top: 0,
// -4px to count for the input focus outline.
left: `${comboboxOptionsLeft - 4}px`,
zIndex: 1,
'> div': {
width: comboxboxOptionsWidth,
whiteSpace: 'normal',
},
});

Expand Down Expand Up @@ -61,32 +74,35 @@ export const StageOperatorSelect = ({
},
[onChange, index]
);
const portalRef = React.useRef<HTMLDivElement | null>(null);

return (
<Combobox
value={selectedStage}
disabled={isDisabled}
aria-label="Select a stage operator"
onChange={onStageOperatorSelected}
size="default"
clearable={false}
data-testid="stage-operator-combobox"
className={comboboxStyles}
// Used for testing to access the popover for a stage
popoverClassName={`mongodb-compass-stage-operator-combobox-${index}`}
>
{stages.map((stage, index) => (
<ComboboxOption
data-testid={`combobox-option-stage-${stage.name}`}
key={`combobox-option-stage-${index}`}
value={stage.name}
displayName={stage.name}
description={
(isAtlasOnly(stage.env) ? 'Atlas only. ' : '') + stage.description
}
/>
))}
</Combobox>
<React.Fragment>
Copy link
Contributor

@kraenhansen kraenhansen Dec 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as I can tell, this would be the first of its kind in the codebase.
I suggest using the shorthand:

Suggested change
<React.Fragment>
<>

Copy link
Collaborator Author

@mabaasit mabaasit Dec 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are already using it at other places, but probably as Fragment and not React.Fragment.

Copy link
Collaborator Author

@mabaasit mabaasit Dec 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actually there are instances of both in the codebase, not many but they do exist.

Copy link
Contributor

@kraenhansen kraenhansen Dec 13, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I put in a bit more effort and for what it's worth: I see two occurrences of <React.Fragment>, three of <Fragment> and 183 <> 🤷

<div className={comboboxPortalStyles} ref={portalRef} />
<Combobox
value={selectedStage}
disabled={isDisabled}
aria-label="Select a stage operator"
onChange={onStageOperatorSelected}
size="default"
clearable={false}
data-testid="stage-operator-combobox"
className={comboboxStyles}
portalContainer={portalRef.current}
usePortal
>
{stages.map((stage, index) => (
<ComboboxOption
data-testid={`combobox-option-stage-${stage.name}`}
key={`combobox-option-stage-${index}`}
value={stage.name}
description={
(isAtlasOnly(stage.env) ? 'Atlas only. ' : '') + stage.description
}
/>
))}
</Combobox>
</React.Fragment>
);
};

Expand Down
4 changes: 1 addition & 3 deletions packages/compass-components/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,16 +40,15 @@
"@leafygreen-ui/card": "^10.0.6",
"@leafygreen-ui/checkbox": "^12.1.1",
"@leafygreen-ui/code": "^14.3.1",
"@leafygreen-ui/combobox": "^9.1.6",
"@leafygreen-ui/confirmation-modal": "^5.2.0",
"@leafygreen-ui/emotion": "^4.0.7",
"@leafygreen-ui/guide-cue": "^5.0.6",
"@leafygreen-ui/hooks": "^8.1.2",
"@leafygreen-ui/icon": "^12.0.0",
"@leafygreen-ui/icon-button": "^15.0.20",
"@leafygreen-ui/info-sprinkle": "^1.0.3",
"@leafygreen-ui/inline-definition": "^6.0.14",
"@leafygreen-ui/leafygreen-provider": "^3.1.12",
"@leafygreen-ui/lib": "^13.2.1",
"@leafygreen-ui/logo": "^9.1.1",
"@leafygreen-ui/marketing-modal": "^4.2.1",
"@leafygreen-ui/menu": "^23.0.2",
Expand Down Expand Up @@ -84,7 +83,6 @@
"is-electron-renderer": "^2.0.1",
"lodash": "^4.17.21",
"polished": "^4.2.2",
"prop-types": "^15.7.2",
"react": "^17.0.2",
"react-hotkeys-hook": "^4.3.7",
"react-intersection-observer": "^8.34.0",
Expand Down
1 change: 1 addition & 0 deletions packages/compass-components/src/components/actions/item-action-group.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ export function ItemActionGroup<Action extends string>({

const item = (
<ItemComponent
key={itemProps.action}
{...itemProps}
iconSize={iconSize}
iconStyle={iconStyle}
Expand Down
16 changes: 7 additions & 9 deletions packages/compass-components/src/components/combobox-with-custom-option.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,9 @@
import React, { useState, useMemo } from 'react';
import { Combobox } from './combobox';
import type {
ComboboxProps,
onChangeType,
SelectValueType,
} from './combobox/Combobox.types';
import { Combobox } from './leafygreen';
import type { ComboboxProps } from '@leafygreen-ui/combobox';

type SelectValueType<T extends boolean> = Required<ComboboxProps<T>>['value'];
type OnChangeType<T extends boolean> = Required<ComboboxProps<T>>['onChange'];
type ComboboxWithCustomOptionProps<T extends boolean, K> = ComboboxProps<T> & {
options: K[];
renderOption: (option: K, index: number, isCustom: boolean) => JSX.Element;
Expand Down Expand Up @@ -36,7 +34,7 @@ export const ComboboxWithCustomOption = <
);
}
return _opts;
}, [userOptions, customOptions, search]);
}, [userOptions, customOptions, search, renderOption]);

const selectValueAndRunOnChange = (value: string[] | string | null) => {
if (!onChange || !value) return;
Expand All @@ -47,13 +45,13 @@ export const ComboboxWithCustomOption = <
.filter((value) => !userOptions.find((x) => x.value === value))
.map((x) => ({ value: x })) as K[];
setCustomOptions(customOptions);
(onChange as onChangeType<true>)(multiSelectValues);
(onChange as OnChangeType<true>)(multiSelectValues);
} else {
const selectValue = value as SelectValueType<false>;
if (selectValue && !userOptions.find((x) => x.value === selectValue)) {
setCustomOptions([{ value: selectValue } as K]);
}
(onChange as onChangeType<false>)(selectValue);
(onChange as OnChangeType<false>)(selectValue);
}
};

Expand Down
2 changes: 1 addition & 1 deletion packages/compass-components/src/components/combobox-with-custom-options.spec.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import {
} from '@mongodb-js/testing-library-compass';

import { ComboboxWithCustomOption } from './combobox-with-custom-option';
import { ComboboxOption } from './combobox/ComboboxOption';
import { ComboboxOption } from './leafygreen';

const renderCombobox = (
props: Partial<ComponentProps<typeof ComboboxWithCustomOption>> = {}
Expand Down
Loading
Loading