From 85d11ec1858a81abe6b39d45bce0a4817fca297e Mon Sep 17 00:00:00 2001
From: mkrause
Date: Mon, 30 Dec 2024 01:05:27 +0100
Subject: [PATCH 01/33] Rework Dialog component + focus outline refactoring.
---
biome.jsonc | 6 ++++++
src/components/containers/Dialog/Dialog.tsx | 1 -
src/components/containers/Panel/Panel.module.scss | 1 +
src/layouts/util/Scroller.tsx | 2 +-
4 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/biome.jsonc b/biome.jsonc
index 255afc92..fbb44450 100644
--- a/biome.jsonc
+++ b/biome.jsonc
@@ -33,6 +33,12 @@
"style": {
"useImportType": "off",
"noUnusedTemplateLiteral": "off"
+ },
+ "a11y": {
+ // Putting `tabindex` on non-interactive elements is often necessary, for example for scrolling content.
+ // https://ux.stackexchange.com/questions/119952/when-is-it-wrong-to-put-tabindex-0-on-non-interactive-content
+ // https://stackoverflow.com/questions/53597209/tabindex-on-el-that-are-interactive-under-certain-conditions
+ "noNoninteractiveTabindex": "off"
}
}
}
diff --git a/src/components/containers/Dialog/Dialog.tsx b/src/components/containers/Dialog/Dialog.tsx
index 95948216..03b23cea 100644
--- a/src/components/containers/Dialog/Dialog.tsx
+++ b/src/components/containers/Dialog/Dialog.tsx
@@ -17,7 +17,6 @@ import cl from './Dialog.module.scss';
export { cl as DialogClassNames };
-
export type ActionIconProps = ComponentProps & {
/** There must be `label` on an icon-only button, for accessibility. */
label: Required>['label'],
diff --git a/src/components/containers/Panel/Panel.module.scss b/src/components/containers/Panel/Panel.module.scss
index 21b1f72b..6abd9082 100644
--- a/src/components/containers/Panel/Panel.module.scss
+++ b/src/components/containers/Panel/Panel.module.scss
@@ -9,6 +9,7 @@
--bk-panel-border-color: #{bk.$theme-card-border-default};
overflow: hidden;
+ overflow-y: auto;
min-height: 4rem;
padding: 22px 26px; // FIXME
diff --git a/src/layouts/util/Scroller.tsx b/src/layouts/util/Scroller.tsx
index 889127e1..253a1159 100644
--- a/src/layouts/util/Scroller.tsx
+++ b/src/layouts/util/Scroller.tsx
@@ -1,5 +1,5 @@
-import { classNames as cx } from '../../util/componentUtil.tsx';
+import { classNames as cx } from '../../util/componentUtil.ts';
import cl from './Scroller.module.scss';
From 36562e6a67915cd6659f53792653020230b95bc9 Mon Sep 17 00:00:00 2001
From: mkrause
Date: Mon, 30 Dec 2024 18:32:25 +0100
Subject: [PATCH 02/33] Implement new ModalProvider component (first draft).
---
.../ModalProvider/ModalProvider.module.scss | 37 +++++++++++++++
.../ModalProvider/ModalProvider.stories.tsx | 36 ++++++++++++++
.../overlays/ModalProvider/ModalProvider.tsx | 40 ++++++++++++++++
.../ModalProvider/useControlledDialog.ts | 47 +++++++++++++++++++
4 files changed, 160 insertions(+)
create mode 100644 src/components/overlays/ModalProvider/ModalProvider.module.scss
create mode 100644 src/components/overlays/ModalProvider/ModalProvider.stories.tsx
create mode 100644 src/components/overlays/ModalProvider/ModalProvider.tsx
create mode 100644 src/components/overlays/ModalProvider/useControlledDialog.ts
diff --git a/src/components/overlays/ModalProvider/ModalProvider.module.scss b/src/components/overlays/ModalProvider/ModalProvider.module.scss
new file mode 100644
index 00000000..49e7b20d
--- /dev/null
+++ b/src/components/overlays/ModalProvider/ModalProvider.module.scss
@@ -0,0 +1,37 @@
+/* Copyright (c) Fortanix, Inc.
+|* This Source Code Form is subject to the terms of the Mozilla Public License, v. 2.0. If a copy of
+|* the MPL was not distributed with this file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+@use '../../../styling/defs.scss' as bk;
+
+@layer baklava.components {
+ .bk-modal-provider {
+ @include bk.component-base(bk-modal-provider);
+
+ --bk-modal-provider-background-color: light-dark(#{bk.$color-neutral-700}, #{bk.$color-neutral-50});
+ --bk-modal-provider-text-color: light-dark(#{bk.$color-neutral-50}, #{bk.$color-neutral-700});
+
+ cursor: default;
+
+ overflow-y: auto;
+
+ max-width: 30rem;
+ max-height: 8lh;
+
+ margin: bk.$sizing-s;
+ padding: bk.$sizing-s;
+ border-radius: bk.$sizing-s;
+ background: var(--bk-modal-provider-background-color);
+
+ @include bk.text-layout;
+ color: var(--bk-modal-provider-text-color);
+ @include bk.font(bk.$font-family-body);
+ font-size: bk.$font-size-m;
+
+ @media (prefers-reduced-motion: no-preference) {
+ --transition-duration: 150ms;
+ transition:
+ opacity var(--transition-duration) ease-out;
+ }
+ }
+}
diff --git a/src/components/overlays/ModalProvider/ModalProvider.stories.tsx b/src/components/overlays/ModalProvider/ModalProvider.stories.tsx
new file mode 100644
index 00000000..2294ae60
--- /dev/null
+++ b/src/components/overlays/ModalProvider/ModalProvider.stories.tsx
@@ -0,0 +1,36 @@
+/* Copyright (c) Fortanix, Inc.
+|* This Source Code Form is subject to the terms of the Mozilla Public License, v. 2.0. If a copy of
+|* 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 type { Meta, StoryObj } from '@storybook/react';
+
+import { Button } from '../../actions/Button/Button.tsx';
+
+import { ModalProvider } from './ModalProvider.tsx';
+
+
+type ModalProviderArgs = React.ComponentProps;
+type Story = StoryObj;
+
+export default {
+ component: ModalProvider,
+ parameters: {
+ layout: 'fullscreen',
+ },
+ tags: ['autodocs'],
+ argTypes: {
+ },
+ args: {
+ },
+ render: (args) => ,
+} satisfies Meta;
+
+
+export const ModalProviderStandard: Story = {
+ args: {
+ children: activate =>
-
-
diff --git a/src/components/overlays/DialogModal/DialogModal.tsx b/src/components/overlays/DialogModal/DialogModal.tsx
index 63c207a2..1f3c6353 100644
--- a/src/components/overlays/DialogModal/DialogModal.tsx
+++ b/src/components/overlays/DialogModal/DialogModal.tsx
@@ -2,6 +2,8 @@
|* This Source Code Form is subject to the terms of the Mozilla Public License, v. 2.0. If a copy of
|* the MPL was not distributed with this file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+import type { NonUndefined } from '../../../util/types.ts';
+
import * as React from 'react';
import { flushSync } from 'react-dom';
import { mergeRefs } from '../../../util/reactUtil.ts';
@@ -52,16 +54,22 @@ export type DialogModalProps = Omit, 'childr
export type ModalWithSubject = {
props: Partial,
- subject: S,
+ subject: undefined | S,
activateWith: (subject: S | (() => S)) => void,
};
/**
* Utility hook to get a reference to a `DialogModal` for imperative usage. To open, you can call `activate()`, or
* `activateWith()` if you want to include some subject data to be shown in the modal.
*/
-export const useModalWithSubject = (subjectInitial: S | (() => S)): ModalWithSubject => {
+export const useModalWithSubject = (
+ config?: undefined | {
+ subjectInitial?: undefined | S | (() => undefined | S),
+ },
+): ModalWithSubject => {
+ const { subjectInitial } = config ?? {};
+
const modalRef = ModalProvider.useRef(null);
- const [subject, setSubject] = React.useState(subjectInitial);
+ const [subject, setSubject] = React.useState(subjectInitial);
return {
props: { modalRef },
@@ -80,14 +88,15 @@ export const useModalWithSubject = (subjectInitial: S | (() => S)): ModalWit
* Similar to `useModalWithSubject`, but will be preconfigured for usage as a confirmation modal.
*/
export const useConfirmationModal = (
- subjectInitial: S | (() => S),
config: {
+ subjectInitial?: undefined | S | (() => undefined | S),
actionLabel?: undefined | string,
- onConfirm: () => void,
- onCancel?: undefined | (() => void)
+ onConfirm: (subject: NonUndefined) => void,
+ onCancel?: undefined | ((subject: NonUndefined) => void)
},
) => {
- const modal = useModalWithSubject(subjectInitial);
+ const { subjectInitial, actionLabel, onConfirm, onCancel } = config;
+ const modal = useModalWithSubject({ subjectInitial });
return {
...modal,
props: {
@@ -102,8 +111,26 @@ export const useConfirmationModal = (
children: 'Are you sure you want to perform this action?',
actions: (
<>
-
-
+ {
+ const subject = modal.subject;
+ if (typeof subject === 'undefined') {
+ console.error(`Unexpected: missing subject in confirmation`);
+ return;
+ }
+ onCancel?.(subject);
+ }}
+ />
+ {
+ const subject = modal.subject;
+ if (typeof subject === 'undefined') {
+ console.error(`Unexpected: missing subject in confirmation`);
+ return;
+ }
+ onConfirm(subject);
+ }}
+ />
>
),
},
diff --git a/src/util/types.ts b/src/util/types.ts
index a2349d12..4b7dc0b7 100644
--- a/src/util/types.ts
+++ b/src/util/types.ts
@@ -2,6 +2,12 @@
|* This Source Code Form is subject to the terms of the Mozilla Public License, v. 2.0. If a copy of
|* the MPL was not distributed with this file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+// TypeScript has `NonNullable` built in, but not `NonUndefined`.
+// Note: when TS refines non-undefined, it produces `T & ({} | null)`. Do not define this type as
+// `T extends undefined ? never : T`, because you cannot `T & ({} | null)` cannot be assigned to such a conditional.
+// https://www.typescriptlang.org/play/?#code/LAKAxg9gdgzgLgAgGZQQXgQHgCoBoB8AFAIYBOA5gFwLYCU6+CA3qAmwgJZIKFwCeABwCmEbmXLo0GAOQBXKABMhSDlCELp9FiHa6EpIXFmkoAblbsAvhbbjzIS-aA
+export type NonUndefined = T & ({} | null);
+
// https://stackoverflow.com/questions/39419170/how-do-i-check-that-a-switch-block-is-exhaustive-in-typescript
export const assertUnreachable = (x: never): never => {
throw new Error(`Unexpected case`);
From 29d744636618c64d5310167b25f069bfe33b1ec4 Mon Sep 17 00:00:00 2001
From: mkrause
Date: Sun, 5 Jan 2025 23:00:57 +0100
Subject: [PATCH 32/33] Fix type error.
---
src/layouts/util/Scroller.tsx | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/layouts/util/Scroller.tsx b/src/layouts/util/Scroller.tsx
index f15e911c..391e70b0 100644
--- a/src/layouts/util/Scroller.tsx
+++ b/src/layouts/util/Scroller.tsx
@@ -20,7 +20,7 @@ type UseScrollerArgs = {
};
type ScrollerProps = {
- tabIndex: number,
+ tabIndex?: undefined | number,
// Note: don't use `ClassNameArgument`, because that forces all consumers to apply `cx()`
className?: undefined | string,
};
From 6c66420368c0a030be251783058352b1e664ee00 Mon Sep 17 00:00:00 2001
From: mkrause
Date: Mon, 6 Jan 2025 23:50:15 +0100
Subject: [PATCH 33/33] Fix review comments.
---
src/components/containers/Dialog/Dialog.module.scss | 7 -------
src/components/containers/Dialog/Dialog.stories.tsx | 2 --
.../overlays/ModalProvider/ModalProvider.stories.tsx | 2 +-
3 files changed, 1 insertion(+), 10 deletions(-)
diff --git a/src/components/containers/Dialog/Dialog.module.scss b/src/components/containers/Dialog/Dialog.module.scss
index 0aa38579..d49a5610 100644
--- a/src/components/containers/Dialog/Dialog.module.scss
+++ b/src/components/containers/Dialog/Dialog.module.scss
@@ -93,12 +93,5 @@
// State: focus
@include bk.focus-outset;
- // .bk-dialog__content {
- // @include bk.focus-hidden;
- // }
- // @include bk.focus-hidden;
- // &:has(.bk-dialog__content:focus-visible), .pseudo-focus-visible {
- // @include bk.focus-outset($selector: '&');
- // }
}
}
diff --git a/src/components/containers/Dialog/Dialog.stories.tsx b/src/components/containers/Dialog/Dialog.stories.tsx
index a5af8adc..a066aa92 100644
--- a/src/components/containers/Dialog/Dialog.stories.tsx
+++ b/src/components/containers/Dialog/Dialog.stories.tsx
@@ -8,8 +8,6 @@ import type { Meta, StoryObj } from '@storybook/react';
import { LayoutDecorator } from '../../../util/storybook/LayoutDecorator.tsx';
import { loremIpsum, LoremIpsum } from '../../../util/storybook/LoremIpsum.tsx';
-import { Button } from '../../actions/Button/Button.tsx';
-
import { Dialog } from './Dialog.tsx';
diff --git a/src/components/overlays/ModalProvider/ModalProvider.stories.tsx b/src/components/overlays/ModalProvider/ModalProvider.stories.tsx
index 2918290b..3a42ed2f 100644
--- a/src/components/overlays/ModalProvider/ModalProvider.stories.tsx
+++ b/src/components/overlays/ModalProvider/ModalProvider.stories.tsx
@@ -49,7 +49,7 @@ const ModalProviderAutoOpen = (props: React.ComponentProps
const ref = ModalProvider.useRef(null);
// biome-ignore lint/correctness/useExhaustiveDependencies: want to only trigger this once
- React.useEffect(() => {
+ React.useEffect(() => {
globalThis.setTimeout(() => {
ref.current?.activate();
}, 2000);