Skip to content

Replace all instances of Dropdown with ListBox, and remove the Dropdown component #173

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

Merged
merged 20 commits into from
May 9, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
32da639
Replace all instances of Dropdown with ListBox, and remove the Dropdo…
mkrause Apr 20, 2025
410f988
Switch to logical properties for overflow: overflow-block/inline.
mkrause Apr 23, 2025
be9c96a
Select: display the human-readable label for the selected option.
mkrause Apr 26, 2025
de348f7
Work on migrating DropdownMenu/Select to use ListBox.
mkrause Apr 30, 2025
c8b132f
ListBox: update styling.
mkrause May 5, 2025
da08176
Fix type errors.
mkrause May 6, 2025
2e0b2aa
Merge branch 'master' into mkrause/250420-replace-dropdown-with-listbox
mkrause May 6, 2025
8e0f4fe
Add missing license header.
mkrause May 6, 2025
767e951
Fix type errors in installation test.
mkrause May 6, 2025
dfc5de0
ListBox: fix 'chin' at the bottom of ListBox.
mkrause May 7, 2025
fcf5954
Investigate GitHub CI issue.
mkrause May 7, 2025
ac4c45c
Merge branch 'master' into mkrause/250420-replace-dropdown-with-listbox
mkrause May 7, 2025
368023d
Fix AppLayout.stories.tsx type errors.
mkrause May 7, 2025
83cd302
ListBox: fix issue where subscribers were not updated when isEmpty st…
mkrause May 7, 2025
19edd59
ListBox: Implement isLoading state.
mkrause May 7, 2025
0f41be9
ListBoxLazy: fix rendering issues + refactor some stories.
mkrause May 8, 2025
a7faad2
ListBoxLazy: fix crash.
mkrause May 8, 2025
9fec639
Remove duplicate check in CI.
mkrause May 9, 2025
58402a1
ListBoxLazy: fix issue with _internalItemsCount.
mkrause May 9, 2025
538346e
SolutionSelector: update props to match AccountSelector.
mkrause May 9, 2025
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
1 change: 0 additions & 1 deletion .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,5 @@ jobs:
cache: 'npm'
- run: npm ci
#- run: npm run build --if-present
- run: npm run verify verify:source
- run: npm test
- run: cd tests/installation && npm ci && npm test
4 changes: 3 additions & 1 deletion .storybook/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,9 @@ const config: StorybookConfig = {
},
framework: {
name: '@storybook/react-vite',
options: {},
options: {
strictMode: true,
},
},
stories: [
'../src/**/*.mdx',
Expand Down
4 changes: 3 additions & 1 deletion .vscode/custom.css-data.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@
"version": 1.1,
"properties": [
{ "name": "interpolate-size" },
{ "name": "position-try-fallbacks" }
{ "name": "position-try-fallbacks" },
{ "name": "reading-flow" },
{ "name": "reading-order" }
],
"atDirectives": [
{
Expand Down
6 changes: 4 additions & 2 deletions app/Demo.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,10 @@ export const Demo = () => {
<Header slot="actions">
<UserMenu userName="Anand Kashyap"/>
{/* <UserMenu userName="Anand Kashyap – Very Long Name That Will Overflow"/> */}
<AccountSelector className="select-action"/>
<SolutionSelector className="select-action"/>
<AccountSelector className="select-action" accounts={null}>
{accountSelected => accountSelected?.label ?? 'Accounts'}
</AccountSelector>
<SolutionSelector className="select-action" solutions={null}/>
</Header>
</AppLayout.Header>
{/* Container around the sidebar that grows to full height, allowing the sidebar to be sticky */}
Expand Down
1 change: 0 additions & 1 deletion app/lib.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,6 @@ export { Tab, Tabs } from '../src/components/navigations/Tabs/Tabs.tsx';
export { SpinnerModal } from '../src/components/overlays/SpinnerModal/SpinnerModal.tsx';
export { DialogModal } from '../src/components/overlays/DialogModal/DialogModal.tsx';
export { DialogOverlay } from '../src/components/overlays/DialogOverlay/DialogOverlay.tsx';
export { DropdownMenu } from '../src/components/overlays/DropdownMenu/DropdownMenu.tsx';
export { DropdownMenuProvider } from '../src/components/overlays/DropdownMenu/DropdownMenuProvider.tsx';
export { ToastProvider, notify } from '../src/components/overlays/ToastProvider/ToastProvider.tsx';
export { Tooltip } from '../src/components/overlays/Tooltip/Tooltip.tsx';
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@
"lint:style": "stylelint 'src/**/*.scss'",
"lint:script": "biome lint",
"lint": "npm run lint:style; npm run lint:script",
"test": "npm run check:types && npm run lint:style",
"test": "npm run check:types && npm run lint:style && npm run verify verify:source",
"test-ui": "vitest --ui",
"coverage": "vitest run --coverage",
"test:storybook": "test-storybook --failOnConsole --browsers chromium --maxWorkers=1",
Expand Down
2 changes: 1 addition & 1 deletion package.json.js
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ const packageConfig = {
// Test
// Note: use `vitest run --root=. src/...` to run a single test file
//'test:unit': 'vitest run --root=.', // Need to specify `--root=.` since the vite root is set to `./app`
'test': 'npm run check:types && npm run lint:style', // TODO: add `lint:script`, `test:unit`
'test': 'npm run check:types && npm run lint:style && npm run verify verify:source', // TODO: add `lint:script`, `test:unit`
'test-ui': 'vitest --ui',
'coverage': 'vitest run --coverage',

Expand Down
4 changes: 4 additions & 0 deletions scripts/import.ts
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,10 @@ const runImportColorsSemantic = async (args: ScriptArgs) => {
throw new Error(`Should not happen`);
}

if (tokenValue.match(/#[a-z0-9]{3,6}/i)) {
throw new Error(`Found semantic color token defined using a hardcoded hex color: '${tokenName}'`);
}

// Replace references to primitive color tokens with their Sass variable equivalent
const color = tokenValue.replaceAll(/var\(--(.+?)-(\d+)\)/g, '\$color-$1-$2');

Expand Down
2 changes: 1 addition & 1 deletion src/components/actions/Button/Button.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ export const Button = (props: ButtonProps) => {
return (
<button
aria-label={accessibleName}
aria-disabled={!isInteractive}
aria-disabled={isInteractive ? undefined : true}
disabled={disabled}
{...propsRest}
type={buttonType} // Not overridable
Expand Down
17 changes: 7 additions & 10 deletions src/components/containers/Banner/Banner.module.scss
Original file line number Diff line number Diff line change
Expand Up @@ -34,26 +34,22 @@
color: bk.$theme-banner-text-default;

.bk-banner__header {
display: flex;
flex-direction: row;
@include bk.flex-row;
align-items: flex-start;
gap: bk.$spacing-4;

.bk-banner__header__text {
margin-inline-end: auto; // Space between text and actions

min-inline-size: 0;
display: flex;
flex-flow: row wrap;
@include bk.flex-row;
flex-wrap: wrap;
gap: bk.$spacing-1;

.bk-banner__title {
$icon-size: 1.3em;
margin-inline-start: calc(-1 * (bk.$spacing-2 + $icon-size));

min-inline-size: 0;
display: flex;
flex-direction: row;
@include bk.flex-row;
gap: bk.$spacing-2;
align-items: center;

Expand All @@ -73,8 +69,9 @@
}
}
.bk-banner__actions {
display: flex;
flex-direction: row;
flex-shrink: 0;

@include bk.flex-row;
align-items: center;
gap: bk.$spacing-3 calc(bk.$spacing-6 / 2);

Expand Down
10 changes: 4 additions & 6 deletions src/components/containers/Dialog/Dialog.module.scss
Original file line number Diff line number Diff line change
Expand Up @@ -67,19 +67,17 @@
margin-block: calc(bk.$spacing-7 / 2) calc(bk.$spacing-7 / 2);
padding-inline: var(--bk-dialog-padding-inline);

display: flex;
flex-direction: row;
gap: bk.$spacing-3;
@include bk.flex($dir: row, $gap: bk.$spacing-3);
}

.bk-dialog__content__icon-aside {
align-self: flex-start;

display: flex;
flex-direction: row;
gap: bk.$spacing-3;
@include bk.flex-row($gap: bk.$spacing-3);
}

.bk-dialog__content__body { --keep: ; }

.bk-dialog__actions {
position: sticky;
inset-block-end: -1px; // -1px to prevent bleed through due to pixel rounding
Expand Down
1 change: 1 addition & 0 deletions src/components/containers/Dialog/Dialog.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ export const DialogWithAside: Story = {
</FieldLayout>
</FormLayout>
</Form>
<p>ThisIsAVeryLongStringOfTextWithoutAnySpacesToTestWhetherWeHandleWordBreaksCorrectlyWhenTheTextOverflowsTheContainingElement</p>
</Dialog>
);
},
Expand Down
2 changes: 1 addition & 1 deletion src/components/containers/Dialog/Dialog.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ export const Dialog = Object.assign(
role="document" // https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Roles/document_role
// FIXME: make this focusable instead of the <dialog> as per guidelines on MDN?
//tabIndex={0}
className={cx('bk-prose')}
className={cx(cl['bk-dialog__content__body'], 'bk-prose')}
>
{children}
</section>
Expand Down
53 changes: 38 additions & 15 deletions src/components/forms/controls/Input/Input.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,9 @@
|* the MPL was not distributed with this file, You can obtain one at http://mozilla.org/MPL/2.0/. */

import * as React from 'react';
import { ClassNameArgument, classNames as cx, type ComponentProps } from '../../../../util/componentUtil.ts';
import { classNames as cx, type ComponentProps } from '../../../../util/componentUtil.ts';
import { mergeRefs, mergeCallbacks } from '../../../../util/reactUtil.ts';
import * as InputUtil from '../../../util/input_util.tsx';

import { type IconName, Icon } from '../../../graphics/Icon/Icon.tsx';
import { IconButton } from '../../../actions/IconButton/IconButton.tsx';
Expand All @@ -29,19 +30,26 @@ const InputAction = (props: React.ComponentProps<typeof IconButton>) => {
);
};

export type InputProps = Omit<ComponentProps<'input'>, 'type'> & {
type InputSpecificProps = Omit<InputUtil.InputSpecificProps, 'type'>;
type InputContainerProps = Omit<ComponentProps<'div'>, 'ref' | keyof InputSpecificProps>;
export type InputProps = InputContainerProps & InputSpecificProps & {
ref?: undefined | React.Ref<HTMLInputElement>,

/** Whether this component should be unstyled. */
unstyled?: undefined | boolean,

/** Class name to apply to the input element. */
inputClassName?: undefined | ClassNameArgument,

/** The type of the input. */
type?: undefined | Exclude<ComponentProps<'input'>['type'], 'button' | 'submit' | 'reset'>,

/** Props to apply to the container element. */
containerProps?: React.ComponentProps<'div'>,

/** Props to apply to the inner `<input/>` element. */
inputProps?: React.ComponentProps<'input'>,

/** An icon to show before the input. */
icon?: undefined | IconName,

/** The accessible name for the icon. */
iconLabel?: undefined | string,

Expand All @@ -53,25 +61,30 @@ export type InputProps = Omit<ComponentProps<'input'>, 'type'> & {
* Note that browser support is still somewhat limited:
* https://developer.mozilla.org/en-US/docs/Web/CSS/field-sizing
*/
automaticResize?: undefined | boolean,
automaticResize?: undefined | boolean,
};
/**
* Input control.
*/
export const Input = Object.assign(
(props: InputProps) => {
const {
unstyled = false,
className,
inputClassName,
ref,
unstyled = false,
type = 'text',
containerProps = {},
inputProps = {},
icon,
iconLabel,
actions,
automaticResize,
...propsRest
} = props;

// Split props into container-specific and input-specific
const propsExtracted = InputUtil.extractInputSpecificProps(propsRest);

const inputRef = React.useRef<HTMLInputElement>(null);

// When the user clicks on the container, focus the input
Expand All @@ -84,8 +97,11 @@ export const Input = Object.assign(
}, []);

// Prevent inputs from being used as (form submit) buttons
//if (type === 'button' || type === 'submit' || type === 'image' || type === 'reset') {
if (['button', 'submit', 'image', 'reset'].includes(type)) {
const bannedTypes = ['button', 'submit', 'image', 'reset'];
if (bannedTypes.includes(type)) {
throw new Error(`Input: unsupported type '${type}'.`);
}
if (inputProps.type && bannedTypes.includes(inputProps.type)) {
throw new Error(`Input: unsupported type '${type}'.`);
}

Expand All @@ -96,20 +112,27 @@ export const Input = Object.assign(

return (
<div
{...containerProps}
{...propsExtracted.containerProps}
className={cx(
'bk',
{ [cl['bk-input']]: !unstyled },
{ [cl['bk-input--automatic-resize']]: automaticResize },
containerProps.className,
propsExtracted.containerProps,
className,
)}
onMouseDown={mergeCallbacks([handleContainerClick, propsRest.onMouseDown])}
onMouseDown={mergeCallbacks(
[handleContainerClick, containerProps.onMouseDown, propsExtracted.containerProps.onMouseDown]
)}
>
{icon && <Icon icon={icon} aria-label={iconLabel}/>}
<input
{...propsRest}
ref={mergeRefs(inputRef, propsRest.ref)}
{...inputProps}
{...propsExtracted.inputProps}
ref={mergeRefs(inputRef, inputProps?.ref, ref)}
type={type}
className={cx(cl['bk-input__input'], inputClassName)}
className={cx(cl['bk-input__input'], inputProps?.className)}
/>
{actions}
</div>
Expand Down
Loading