fix: changed name from alert to prompt, fixed small nit, added aria-l…

…abelled as an example
dequelabs · Aug 29, 2023 · 86aaa10 · 86aaa10
1 parent 92d725d
commit 86aaa10
Show file tree

Hide file tree

Showing 5 changed files with 45 additions and 42 deletions.
diff --git 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!
       </Button>
       <Popover
-        variant="alert"
+        variant="prompt"
         target={buttonRef} 
         placement="auto" 
         show={show} 
@@ -54,7 +54,7 @@ function TextTooltipExamples() {
 For custom purposes you can use "custom" variant, which is default.
 
 ```jsx example
-function BigTooltipExample() {
+function CustomPopoverExample() {
   const [show, setShow] = useState(false);
   const onTogglePopover = () => setShow(!show);
   const buttonRef = useRef();
@@ -73,8 +73,10 @@ function BigTooltipExample() {
         placement="auto" 
         show={show} 
         onClose={onTogglePopover}
+        aria-labelledby={"popover-title"}
       >
         <div>
+          <h2 id="popover-title">Popover Title</h2>
           <p>Popover content</p>
           <Button>Custom button</Button>
           <Button>Custom button</Button>
@@ -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.'
     },
   ]}
 />
diff --git 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
       </button>
       <Popover
-        variant="alert"
+        variant="prompt"
         target={ref}
         show
         onClose={onClose}
@@ -193,8 +193,8 @@ test('first element inside the popover container should have focus', async () =>
   expect(firstInteractableElement.instance()).toBe(focusedElement);
 });
 
-test('should render two buttons (Apply/Close) for alert variant', async () => {
-  const wrapper = mount(<Wrapper tooltipProps={{ variant: 'alert' }} />, {
+test('should render two buttons (Apply/Close) for prompt variant', async () => {
+  const wrapper = mount(<Wrapper tooltipProps={{ variant: 'prompt' }} />, {
     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(
-    <WrapperAlert tooltipProps={{ variant: 'alert', onClose: handleClose }} />,
+    <WrapperPrompt
+      tooltipProps={{ variant: 'prompt', onClose: handleClose }}
+    />,
     { 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(
-    <WrapperAlert tooltipProps={{ variant: 'alert', onApply: applyFunc }} />,
+    <WrapperPrompt tooltipProps={{ variant: 'prompt', onApply: applyFunc }} />,
     { 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(
-    <WrapperAlert
-      tooltipProps={{ variant: 'alert', closeButtonText, applyButtonText }}
+    <WrapperPrompt
+      tooltipProps={{ variant: 'prompt', closeButtonText, applyButtonText }}
     />,
     { 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(<WrapperAlert />);
+test('variant="prompt" should return no axe violations', async () => {
+  const wrapper = mount(<WrapperPrompt />);
   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(<WrapperAlert />);
+test('aria-labelledby is set correctly for prompt variant', async () => {
+  const wrapper = mount(<WrapperPrompt />);
   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
@@ -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<HTMLDivElement> & {
   target: React.RefObject<HTMLElement> | 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<HTMLDivElement, PopoverProps>(
       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<HTMLDivElement, PopoverProps>(
               className,
               {
                 'Popover--hidden': !show,
-                'Popover--alert': variant === 'alert'
+                'Popover--prompt': variant === 'prompt'
               }
             )}
             ref={popoverRef}
@@ -262,8 +262,8 @@ const Popover = forwardRef<HTMLDivElement, PopoverProps>(
               style={styles.arrow}
             />
             <div className="Popover__borderLeft" />
-            {variant === 'alert' ? (
-              <AlertPopoverContent
+            {variant === 'prompt' ? (
+              <PromptPopoverContent
                 applyButtonText={applyButtonText}
                 onApply={onApply}
                 closeButtonText={closeButtonText}

diff --git a/packages/react/src/index.ts b/packages/react/src/index.ts
@@ -129,7 +129,6 @@ export {
 
 export { default as Popover } from './components/Popover';
 
-
 /**
  * Helpers / Utils
  */

diff --git a/packages/styles/popover.css b/packages/styles/popover.css
@@ -2,7 +2,7 @@
   --popover-background-color: var(--gray-10);
   --popover-border-color: var(--gray-90);
   --popover-text-color: var(--gray-90);
-  --popover-box-shadow: rgba(0, 0, 0, 0.35) 0px 0px 10px 0px;
+  --popover-box-shadow: rgba(0, 0, 0, 0.35) 0 0 10px 0;
   --popover-font-size: var(--text-size-smaller);
   --popover-line-height: var(--text-size-normal);
   --popover-padding: 0 var(--space-small);