Skip to content

Commit

Permalink
fix: Avoid submitting form when MultiSelect state changes
Browse files Browse the repository at this point in the history
Signed-off-by: Noble Mittal <noblemittal@outlook.com>
  • Loading branch information
beingnoble03 committed Sep 14, 2024
1 parent 3084278 commit 03303b1
Show file tree
Hide file tree
Showing 6 changed files with 94 additions and 113 deletions.
1 change: 1 addition & 0 deletions web/vtadmin/src/components/TextInput.module.scss
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ $iconPositionHorizontalLarge: 1.6rem;
&:disabled {
background: var(--backgroundSecondary);
border-color: var(--backgroundSecondaryHighlight);
color: var(--colorDisabled);
cursor: not-allowed;
}

Expand Down
2 changes: 1 addition & 1 deletion web/vtadmin/src/components/dropdown/MenuItem.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ const MenuItem: React.FC<MenuItemProps> = ({ children, disabled, className, inte
<button
disabled={disabled}
onMouseDown={(e) => e.preventDefault()}
className={`transition-colors font-sans border-none text-left block px-6 py-4 disabled:bg-gray-100 hover:bg-gray-100 text-${intent} hover:text-${
className={`transition-colors font-sans border-none text-left block px-6 py-4 disabled:bg-gray-200 disabled:text-secondary hover:bg-gray-100 text-${intent} hover:text-${
intent === Intent.none ? 'vtblue' : intent
} w-full ${className || ''}`}
role="menuitem"
Expand Down
63 changes: 3 additions & 60 deletions web/vtadmin/src/components/inputs/MultiSelect.module.scss
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
.container {
.container {
display: inline-block;
position: relative;
}
Expand Down Expand Up @@ -55,13 +55,6 @@
cursor: not-allowed;
}

.large .toggle {
font-size: theme('fontSize.lg');
height: var(--inputHeightLarge);
min-width: 24rem;
padding: 0 16px;
}

.chevron {
height: 20px;
position: absolute;
Expand All @@ -85,14 +78,7 @@
z-index: 1000;
}

.menu {
list-style-type: none;
margin: 0;
outline: none;
padding: 0;
}

.menuItem {
.listItem {
background: none;
border: none;
box-sizing: border-box;
Expand All @@ -114,49 +100,6 @@
background: var(--backgroundPrimaryHighlight);
cursor: pointer;
}

&:active,
&:focus {
background: var(--backgroundPrimaryHighlight);
}
}

.clear {
background: none;
border: none;
box-sizing: border-box;
color: var(--textColorSecondary);
cursor: pointer;
display: block;
font-family: var(--fontFamilyPrimary);
font-size: var(--fontSizeBody);
min-width: 16rem;
padding: 4px 12px;
position: relative;
text-align: left;
transition: all 0.1s ease-in-out;
white-space: nowrap;
width: 100%;

&:hover,
&:active,
&:focus {
background: var(--backgroundPrimaryHighlight);
}
}

.large .clear {
font-size: theme('fontSize.lg');
padding: 8px 16px;
}

.emptyContainer {
outline: none;
padding: 8px 12px;
}

.emptyPlaceholder {
color: var(--textColorSecondary);
}

.selected {
Expand All @@ -165,4 +108,4 @@
&:hover {
background: var(--backgroundSecondaryHighlight);
}
}
}
28 changes: 12 additions & 16 deletions web/vtadmin/src/components/inputs/MultiSelect.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ interface Props<T> {
items: T[];
itemToString?: (item: T) => string;
label: string;
setSelectedItems: (selectedItems: T[]) => void;
onChange: (selectedItems: T[]) => void;
placeholder: string;
renderItem?: (item: T) => JSX.Element | string;
selectedItems: T[];
Expand All @@ -45,7 +45,7 @@ export const MultiSelect = <T,>({
placeholder,
renderItem,
selectedItems,
setSelectedItems,
onChange,
description,
required,
}: Props<T>) => {
Expand All @@ -56,42 +56,38 @@ export const MultiSelect = <T,>({
return itemToString(item);
};

const selectedItemsText = `Selected (${selectedItems.length}): ${selectedItems.map((item) => _renderItem(item)).join(', ')}`
const selectedItemsText = `Selected (${selectedItems.length}): ${selectedItems.map((item) => _renderItem(item)).join(', ')}`;

return (
<div className={cx(style.container, className, { [style.disabled]: disabled })}>
{label && (
<Label label={label} required={required} />
)}
{label && <Label label={label} required={required} />}
{description && <div className="mt-[-4px] mb-4">{description}</div>}

<Listbox value={selectedItems} onChange={setSelectedItems} multiple>
<Listbox value={selectedItems} onChange={onChange} multiple>
{({ open }) => (
<>
<Listbox.Button
className={cx(style.toggle, inputClassName, { [style.open]: open, [style.disabled]: disabled })}
className={cx(style.toggle, inputClassName, {
[style.open]: open,
[style.disabled]: disabled,
})}
>
{selectedItems.length > 0
? selectedItemsText
: placeholder}
{selectedItems.length > 0 ? selectedItemsText : placeholder}
<Icon className={style.chevron} icon={open ? Icons.chevronUp : Icons.chevronDown} />
</Listbox.Button>
<Listbox.Options className={cx(style.dropdown, { [style.hidden]: !open })}>
{items.map((item, index) => (
<Listbox.Option
as="button"
key={index}
value={item}
className={({ active, selected }) =>
cx(style.menuItem, {
cx(style.listItem, {
[style.active]: active,
[style.selected]: selected,
})
}
>
<div>
{_renderItem(item)}
</div>
<div>{_renderItem(item)}</div>
</Listbox.Option>
))}
</Listbox.Options>
Expand Down
4 changes: 3 additions & 1 deletion web/vtadmin/src/components/routes/Workflows.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -196,9 +196,11 @@ export const Workflows = () => {
<ReadOnlyGate>
<div>
<Dropdown dropdownButton={Icons.circleAdd} position="bottom-right">
<Link to="/workflows/move_tables/create">
<MenuItem>
<Link to="/workflows/move_tables/create">Move Tables</Link>
Move Tables
</MenuItem>
</Link>
<MenuItem disabled>Reshard</MenuItem>
<MenuItem disabled>Materialize</MenuItem>
</Dropdown>
Expand Down
Loading

0 comments on commit 03303b1

Please sign in to comment.