FP-990 & FP-1255: Update <IntroMessage> to Use Dismissible <SectionMe…

…ssage> (#616) * FP-990: Fix bug of info Message missing X button @swillrich5 found and fixed this bug. I replicate the fix so I can test my FP-990 solution. * FP-990: Add some examples for SectionMessage usage * FP-990: <Message type=scope>, not <SectionMessage> I couldn't figure out how to use `<Sectionmessage>` to achieve FP-990. * FP-990: Fix failing ManageAccount test * FP-990: (CSS) Separate SectionMessage form content * chore(lint): run prettier:fix * FP-990: Fix Ability to Dimiss SectionMessages * Quick: Fix IntroMessage Spacing (simple cause during migration to vite) * Quick: In IntroMessage Use SectionMessage * Quick: Update Message Components JSDocs * FP-990: More Fix Ability to Dimiss SectionMessages * chore(prettier): "fix" whitespace * FP-990: New Colors for Info Messages I know I break no other styles because only Message...css used these. * FP-1255: Fix bug that has not manifested * fix(fp-990): use const not let * fix(fp-990): remove leftover setting of onDismiss * fix(fp-990): undefine props w/out new unused vars Set props undefined, instead of removing and creating two unsued vars. * fix(fp-990): prettier fix Co-authored-by: Sal Tijerina <r.sal.tijerina@gmail.com>
TACC · May 4, 2022 · b8bf18b · b8bf18b
1 parent 5adcb06
commit b8bf18b
Show file tree

Hide file tree

Showing 10 changed files with 92 additions and 44 deletions.
diff --git a/client/src/components/ManageAccount/tests/ManageAccount.test.js b/client/src/components/ManageAccount/tests/ManageAccount.test.js
@@ -37,8 +37,6 @@ describe('Manage Account Page', () => {
     expect(getByText(/Manage Account/)).toBeInTheDocument();
     expect(getByText(/Back to Dashboard/)).toBeInTheDocument();
     expect(getAllByText(/Loading.../)).toBeDefined();
-    expect(
-      getByRole('alert', { class: /introMessageGeneral/i })
-    ).toBeInTheDocument();
+    expect(getByRole('status', { name: 'message' })).toBeInTheDocument();
   });
 });
diff --git a/client/src/components/_common/InlineMessage/InlineMessage.jsx b/client/src/components/_common/InlineMessage/InlineMessage.jsx
@@ -2,11 +2,11 @@ import React from 'react';
 import Message from '_common/Message';
 
 /**
- * Show an event-based message to the user
- * @todo Document examples
+ * Show a component-specific event-based message to the user
  * @example
- * // Blah blah…
- * <Sample jsx>
+ * // basic usage
+ * <InlineMessage type="success">Task complete.</InlineMessage>
+ * @see _common/Message
  */
 const InlineMessage = (props) => {
   // Override default props

diff --git a/client/src/components/_common/IntroMessage/IntroMessage.jsx b/client/src/components/_common/IntroMessage/IntroMessage.jsx
@@ -1,8 +1,11 @@
-import React from 'react';
+import React, { useCallback, useState } from 'react';
 import PropTypes from 'prop-types';
-import { Alert } from 'reactstrap';
 import { useDispatch, useSelector } from 'react-redux';
 
+import { SectionMessage } from '_common';
+
+import styles from './IntroMessage.module.css';
+
 /**
  * Whether the name is of a known intro message
  * @param {String} messageName - The name of the message to check
@@ -31,24 +34,30 @@ function IntroMessage({ children, className, messageName }) {
   const dispatch = useDispatch();
   const introMessages = useSelector((state) => state.introMessages);
   const shouldShow = isKnownMessage(messageName);
+  const [isVisible, setIsVisible] = useState(shouldShow);
 
-  function onDismiss(name) {
+  // Manage visibility
+  const onDismiss = useCallback(() => {
     const newMessagesState = {
       ...introMessages,
-      [name]: false,
+      [messageName]: false,
     };
     dispatch({ type: 'SAVE_INTRO', payload: newMessagesState });
-  }
+
+    setIsVisible(!isVisible);
+  }, [isVisible]);
 
   return (
-    <Alert
-      isOpen={shouldShow}
-      toggle={() => onDismiss(messageName)}
-      color="secondary"
-      className={className}
+    <SectionMessage
+      aria-label={messageName}
+      type="info"
+      canDismiss
+      className={`${styles.root} ${className}`}
+      isVisible={isVisible}
+      onDismiss={onDismiss}
     >
       {children}
-    </Alert>
+    </SectionMessage>
   );
 }
 IntroMessage.propTypes = {

diff --git a/client/src/components/_common/IntroMessage/IntroMessage.module.css b/client/src/components/_common/IntroMessage/IntroMessage.module.css
@@ -0,0 +1,6 @@
+.root {
+  /* To replicate margin that is unset by `<Message>` */
+  /* HACK: Overwrite spacing that was removed */
+  /* FP-1227: Standardize Message margin and do not hack here */
+  margin: var(--global-space--section-top) 0 !important;
+}
diff --git a/client/src/components/_common/IntroMessage/IntroMessage.test.js b/client/src/components/_common/IntroMessage/IntroMessage.test.js
@@ -20,7 +20,7 @@ describe('IntroMessage', () => {
       );
       expect(container.getElementsByClassName('test-class').length).toEqual(1);
       // NOTE: The `status` role (https://www.w3.org/TR/html-aria/#index-aria-status) is more appropriate than the `alert` role (https://www.w3.org/TR/html-aria/#index-aria-alert), but setting the `role` attribute of an `Alert` is ineffectual
-      expect(getByRole('alert')).not.toEqual(null);
+      expect(getByRole('status')).not.toEqual(null);
       expect(getByText('Test Message')).not.toEqual(null);
     });
   });

diff --git a/client/src/components/_common/Message/Message.jsx b/client/src/components/_common/Message/Message.jsx
@@ -57,12 +57,30 @@ export const DEFAULT_SCOPE = 'inline'; // FAQ: Historical support for default
 
 /**
  * Show an event-based message to the user
- * @todo Document examples
  * @example
- * // Blah blah…
- * <Sample jsx>
+ * // basic usage
+ * <Message type="error" scope="inline">Invalid content.</Message>
+ * @example
+ * // manage dismissal and visibility
+ * const [isVisible, setIsVisible] = useState(...);
+ *
+ * const onDismiss = useCallback(() => {
+ *   setIsVisible(!isVisible);
+ * }, [isVisible]);
+ *
+ * return (
+ *   <SectionMessage
+ *     type="warning"
+ *     isVisible={isVisible}
+ *     onDismiss={onDismiss}
+ *   >
+ *     Uh oh.
+ *   </SectionMessage>
+ * );
+ * ...
  */
 const Message = ({
+  ariaLabel,
   children,
   className,
   dataTestid,
@@ -123,6 +141,7 @@ const Message = ({
       className={`${className} ${containerStyleNames}`}
       role={role}
       in={isVisible}
+      aria-label={ariaLabel}
       data-testid={dataTestid}
     >
       <Icon
@@ -151,6 +170,8 @@ const Message = ({
   );
 };
 Message.propTypes = {
+  /** How to label this message for accessibility (via `aria-label`) */
+  ariaLabel: PropTypes.string,
   /** Whether an action can be dismissed (requires scope equals `section`) */
   canDismiss: PropTypes.bool,
   /** Message text (as child node) */
@@ -170,6 +191,7 @@ Message.propTypes = {
   type: PropTypes.oneOf(TYPES).isRequired,
 };
 Message.defaultProps = {
+  ariaLabel: 'message',
   className: '',
   canDismiss: false,
   dataTestid: '',

diff --git a/client/src/components/_common/Message/Message.module.scss b/client/src/components/_common/Message/Message.module.scss
@@ -27,6 +27,7 @@
 
   display: flex;
 }
+/* HELP: FP-1227: Why is this unset? */
 p.is-scope-section {
   margin-top: 0;
   margin-bottom: 0;
@@ -61,15 +62,15 @@ p.is-scope-section {
 /* Modifiers: Type */
 
 /* Design decided icon is not necessary for informational messages */
-.is-info .icon {
+.is-info .icon:not(.close-icon) {
   display: none;
 }
 
 /* Modifiers: Scope */
 
 .is-scope-inline {
   &.is-info .icon {
-    color: var(--global-color-info--normal);
+    color: var(--global-color-info--dark);
   }
   &.is-warn .icon {
     color: var(--global-color-warning--normal);
@@ -94,10 +95,11 @@ p.is-scope-section {
 
   /* Modifiers */
   &.is-info {
+    color: var(--global-color-info--dark);
     border-color: var(--global-color-info--normal);
-    background-color: var(--global-color-info--weak);
+    background-color: var(--global-color-info--light);
     & .type-icon {
-      color: var(--global-color-info--normal);
+      color: var(--global-color-info--dark);
     }
   }
   &.is-warn {

diff --git a/client/src/components/_common/Section/SectionMessages.jsx b/client/src/components/_common/Section/SectionMessages.jsx
@@ -54,7 +54,7 @@ function SectionMessages({
   const introMessageContent = introMessageText || MESSAGES[introMessageName];
   const introMessage = introMessageContent && (
     /* FAQ: Alternate message name allows tracking custom message dismissal */
-    <IntroMessage messageName={introMessageName || introMessageText}>
+    <IntroMessage messageName={introMessageName || introMessageText} canDismiss>
       {introMessageContent}
     </IntroMessage>
   );

diff --git a/client/src/components/_common/SectionMessage/SectionMessage.jsx b/client/src/components/_common/SectionMessage/SectionMessage.jsx
@@ -3,38 +3,48 @@ import PropTypes from 'prop-types';
 import Message from '_common/Message';
 
 /**
- * Show an event-based message to the user
- * @todo Document examples
+ * Show a section/page-specific event-based message to the user
  * @example
- * // Blah blah…
- * <Sample jsx>
+ * // basic usage
+ * <SectionMessage type="warning">Uh oh.</SectionMessage>
+ * @see _common/Message
  */
 const SectionMessage = (props) => {
   const [isVisible, setIsVisible] = useState(true);
+  const autoManageVisible = props.canDismiss && props.isVisible === undefined;
+  const autoManageDismiss = props.canDismiss && props.onDismiss === undefined;
 
-  // Manage visibility
-  const onDismiss = useCallback(() => {
-    setIsVisible(!isVisible);
-  }, [isVisible]);
+  function onDismiss() {
+    if (autoManageVisible) {
+      setIsVisible(!isVisible);
+    }
+    if (!autoManageDismiss) {
+      props.onDismiss();
+    }
+  }
 
   // Override default props
   const messageProps = {
     ...Message.defaultProps,
     ...props,
-    isVisible,
-    onDismiss,
     scope: 'section',
   };
+  if (autoManageVisible) {
+    messageProps.isVisible = isVisible;
+  }
+  if (autoManageDismiss) {
+    messageProps.onDismiss = onDismiss;
+  }
 
   // Avoid manually syncing <Message>'s props
   // eslint-disable-next-line react/jsx-props-no-spreading
   return <Message {...messageProps} />;
 };
-SectionMessage.propTypes = {
-  ...Message.propTypes,
-  isVisible: PropTypes.bool,
-  onDismiss: PropTypes.func,
+SectionMessage.propTypes = Message.propTypes;
+SectionMessage.defaultProps = {
+  ...Message.defaultProps,
+  isVisible: undefined,
+  onDismiss: undefined,
 };
-SectionMessage.defaultProps = Message.defaultProps;
 
 export default SectionMessage;
diff --git a/client/src/styles/settings/color.css b/client/src/styles/settings/color.css
@@ -40,8 +40,9 @@
   --global-color-accent--weak: #7754e840; /* =25% opacity */
   /* on white, eq. to #DFD7FA */
   --global-color-accent--alt: #d2cce7;
-  --global-color-info--normal: var(--global-color-primary--x-dark);
-  --global-color-info--weak: var(--global-color-primary--x-light);
+  --global-color-info--dark: var(--global-color-primary--x-dark);
+  --global-color-info--normal: var(--global-color-primary--normal);
+  --global-color-info--light: var(--global-color-primary--x-light);
   --global-color-success--normal: #43d130;
   --global-color-success--weak: #43d13020;
   --global-color-success--alt: var(--global-color-warning--normal);