From 86aaa10cd131a6d3884f021aae3938988f2427ad Mon Sep 17 00:00:00 2001 From: orest-s Date: Mon, 28 Aug 2023 11:19:24 +0300 Subject: [PATCH] fix: changed name from alert to prompt, fixed small nit, added aria-labelled as an example --- docs/pages/components/Popover.mdx | 28 +++++++++-------- .../__tests__/src/components/Popover/index.js | 30 ++++++++++--------- .../react/src/components/Popover/index.tsx | 26 ++++++++-------- packages/react/src/index.ts | 1 - packages/styles/popover.css | 2 +- 5 files changed, 45 insertions(+), 42 deletions(-) diff --git a/docs/pages/components/Popover.mdx b/docs/pages/components/Popover.mdx index c5d686b97..c920255ff 100644 --- a/docs/pages/components/Popover.mdx +++ b/docs/pages/components/Popover.mdx @@ -15,14 +15,14 @@ import { Popover, Button } from '@deque/cauldron-react' Cauldron's tooltip relies on [Popper](https://popper.js.org/) to position tooltips dynamically. Popover can be triggered from any focusable element via a `target` attribute pointed to an HTMLElement or React ref object. -### Alert Popover +### Prompt Popover -Alert Popovers are designed to display a popup with two buttons, the first to apply some action, the second to cancel, +Prompt Popovers are designed to display a popup with two buttons, the first to apply some action, the second to cancel, also popup contains information text to show some message to the user. Canceling the action will only close the popup window. ```jsx example -function TextTooltipExamples() { +function PromptPopoverExample() { const [show, setShow] = useState(false); const onTogglePopover = () => setShow(!show); const buttonRef = useRef(); @@ -34,10 +34,10 @@ function TextTooltipExamples() { variant="secondary" buttonRef={buttonRef} > - Alert! + Prompt! setShow(!show); const buttonRef = useRef(); @@ -73,8 +73,10 @@ function BigTooltipExample() { placement="auto" show={show} onClose={onTogglePopover} + aria-labelledby={"popover-title"} >
+

Popover Title

Popover content

@@ -116,13 +118,13 @@ function BigTooltipExample() { name: 'infoText', required: true, type: 'string', - description: 'Information text to show some warning or useful information in Alert variant of the component. Please note that required is only for "Alert" variant.', + description: 'Information text to show some warning or useful information in Prompt variant of the component. Please note that required is only for "Prompt" variant.', }, { name: 'onApply', required: true, type: 'function', - description: 'Callback, which is used for Apply button in Alert variant of the component. Please note, that is only required for Alert variant', + description: 'Callback, which is used for Apply button in Prompt variant of the component. Please note, that is only required for Prompt variant', }, { name: 'placement', @@ -132,7 +134,7 @@ function BigTooltipExample() { }, { name: 'variant', - type: ['custom', 'alert'], + type: ['custom', 'prompt'], defaultValue: 'custom', description: 'The style of Popover to display.' }, @@ -151,24 +153,24 @@ function BigTooltipExample() { { name: 'applyButtonText', type: 'string', - description: 'Label for button, which is used to apply action in Alert variant of the popover', + description: 'Label for button, which is used to apply action in Prompt variant of the popover', defaultValue: 'Apply' }, { name: 'closeButtonText', type: 'string', - description: 'Label for button, which is used to close popover in Alert variant of the component', + description: 'Label for button, which is used to close popover in Prompt variant of the component', defaultValue: 'Close' }, { name: 'aria-label', type: 'string', - description: 'A label for tabs is required. This means you must provide either an aria-label or aria-labelledby prop. Please note that for alert variant we use infoText to get labelled by' + description: 'A label for Popover is required for non-custom variants. This means you must provide either an aria-label or aria-labelledby prop.' }, { name: 'aria-labelledby', type: 'string', - description: 'A label for tabs is required. This means you must provide either an aria-label or aria-labelledby prop. Please note that for alert variant we use infoText to get labelled by' + description: 'A label for Popover is required for non-custom variants. This means you must provide either an aria-label or aria-labelledby prop.' }, ]} /> \ No newline at end of file diff --git a/packages/react/__tests__/src/components/Popover/index.js b/packages/react/__tests__/src/components/Popover/index.js index d0673e84b..746f7a01b 100644 --- a/packages/react/__tests__/src/components/Popover/index.js +++ b/packages/react/__tests__/src/components/Popover/index.js @@ -65,7 +65,7 @@ const WrapperPopoverWithElements = () => { }; // eslint-disable-next-line react/prop-types -const WrapperAlert = ({ buttonProps = {}, tooltipProps = {} }) => { +const WrapperPrompt = ({ buttonProps = {}, tooltipProps = {} }) => { const ref = useRef(); const onClose = jest.fn(); return ( @@ -74,7 +74,7 @@ const WrapperAlert = ({ buttonProps = {}, tooltipProps = {} }) => { button expect(firstInteractableElement.instance()).toBe(focusedElement); }); -test('should render two buttons (Apply/Close) for alert variant', async () => { - const wrapper = mount(, { +test('should render two buttons (Apply/Close) for prompt variant', async () => { + const wrapper = mount(, { attachTo: mountNode }); @@ -207,11 +207,13 @@ test('should render two buttons (Apply/Close) for alert variant', async () => { expect(applyBtn.exists()).toBeTruthy(); }); -test('onClose should be called, when close button in alert popover is clicked', async () => { +test('onClose should be called, when close button in prompt popover is clicked', async () => { const handleClose = jest.fn(); const wrapper = mount( - , + , { attachTo: mountNode } ); @@ -222,11 +224,11 @@ test('onClose should be called, when close button in alert popover is clicked', expect(handleClose).toHaveBeenCalled(); }); -test('onApply should be called, when apply button in alert popover is clicked', async () => { +test('onApply should be called, when apply button in prompt popover is clicked', async () => { const applyFunc = jest.fn(); const wrapper = mount( - , + , { attachTo: mountNode } ); @@ -242,8 +244,8 @@ test('text for apply/close buttons are rendered correct', async () => { const applyButtonText = 'Specific text to apply popover'; const wrapper = mount( - , { attachTo: mountNode } ); @@ -258,8 +260,8 @@ test('text for apply/close buttons are rendered correct', async () => { expect(applyBtn.text()).toBe(applyButtonText); }); -test('variant="alert" should return no axe violations', async () => { - const wrapper = mount(); +test('variant="prompt" should return no axe violations', async () => { + const wrapper = mount(); await update(wrapper); expect(await axe(wrapper.html())).toHaveNoViolations(); }); @@ -332,8 +334,8 @@ test('deactivates aria isolate on hide', async () => { expect(deactivateFn).toBeCalled(); }); -test('aria-labelledby is set correctly for alert variant', async () => { - const wrapper = mount(); +test('aria-labelledby is set correctly for prompt variant', async () => { + const wrapper = mount(); await update(wrapper); const id = wrapper.find('.Popover').props().id; diff --git a/packages/react/src/components/Popover/index.tsx b/packages/react/src/components/Popover/index.tsx index f678bc571..6fdcacffd 100644 --- a/packages/react/src/components/Popover/index.tsx +++ b/packages/react/src/components/Popover/index.tsx @@ -14,7 +14,7 @@ import focusableSelector from '../../utils/focusable-selector'; import AriaIsolate from '../../utils/aria-isolate'; import useSharedRef from '../../utils/useSharedRef'; -export type PopoverVariant = 'alert' | 'custom'; +export type PopoverVariant = 'prompt' | 'custom'; type BaseProps = React.HTMLAttributes & { target: React.RefObject | HTMLElement; @@ -30,22 +30,22 @@ type CustomProps = BaseProps & { applyButtonText?: string; onApply?: () => void; closeButtonText?: string; - infoText?: string; - chidlren: ReactNode; + infoText?: ReactNode; + children: ReactNode; } & Cauldron.LabelProps; -type AlertProps = BaseProps & { - variant: 'alert'; +type PromptProps = BaseProps & { + variant: 'prompt'; applyButtonText?: string; onApply: () => void; closeButtonText?: string; - infoText: string; - chidlren?: ReactNode; + infoText: ReactNode; + children?: ReactNode; }; -export type PopoverProps = AlertProps | CustomProps; +export type PopoverProps = PromptProps | CustomProps; -const AlertPopoverContent = ({ +const PromptPopoverContent = ({ onClose, applyButtonText = 'Apply', onApply, @@ -126,7 +126,7 @@ const Popover = forwardRef( initialPlacement; const additionalProps = - variant === 'alert' ? { 'aria-labelledby': `${id}-label` } : {}; + variant === 'prompt' ? { 'aria-labelledby': `${id}-label` } : {}; // Keep targetElement in sync with target prop useEffect(() => { @@ -246,7 +246,7 @@ const Popover = forwardRef( className, { 'Popover--hidden': !show, - 'Popover--alert': variant === 'alert' + 'Popover--prompt': variant === 'prompt' } )} ref={popoverRef} @@ -262,8 +262,8 @@ const Popover = forwardRef( style={styles.arrow} />
- {variant === 'alert' ? ( -