From fec33741e0e3203aeebcc0af4dff8b4303732e0f Mon Sep 17 00:00:00 2001 From: Filip Hlavac <50696716+fhlavac@users.noreply.github.com> Date: Thu, 18 Apr 2024 10:25:57 +0200 Subject: [PATCH] Improve OUIA id's in current components (#131) * Improve ouiaIds in Ansible * Improve ouiaIds in Battery * Improve ouiaIds in CloseButton * Improve ouiaIds in Error components * Improve ouiaIds in state components * Improve ouiaIds in Shortcut * Improve ouiaIds in WarningModal * Improve ouiaIds in SkeletonTable * Improve ouiaIds in MultiContentCard * feat(ouia): Improve ouiaIds in LogSnippet & TagCount * Update the contribution guide --- README.md | 1 + packages/module/src/Ansible/Ansible.tsx | 8 +- .../__snapshots__/Ansible.test.tsx.snap | 4 + packages/module/src/Battery/Battery.tsx | 6 +- .../__snapshots__/Batery.test.tsx.snap | 6 + .../module/src/CloseButton/CloseButton.tsx | 2 + .../src/ErrorBoundary/ErrorBoundary.tsx | 25 ++-- packages/module/src/ErrorState/ErrorState.tsx | 18 +-- .../src/InvalidObject/InvalidObject.tsx | 35 +++--- .../__snapshots__/InvalidObject.test.tsx.snap | 14 ++- packages/module/src/LogSnippet/LogSnippet.tsx | 12 +- .../__snapshots__/LogSnippet.test.tsx.snap | 12 +- .../src/MultiContentCard/MultiContentCard.tsx | 16 ++- .../MultiContentCard.test.tsx.snap | 31 ++++- .../src/NotAuthorized/NotAuthorized.tsx | 15 ++- .../__snapshots__/NotAuthorized.test.tsx.snap | 45 ++++++- .../module/src/NotFoundIcon/NotFoundIcon.tsx | 3 +- packages/module/src/Shortcut/Shortcut.tsx | 8 +- .../__snapshots__/Shortcut.test.tsx.snap | 2 + .../module/src/ShortcutGrid/ShortcutGrid.tsx | 12 +- .../__snapshots__/ShortcutGrid.test.tsx.snap | 20 +++ .../src/SkeletonTable/SkeletonTable.tsx | 26 ++-- .../__snapshots__/SkeletonTable.test.tsx.snap | 116 +++++++++++++----- packages/module/src/TagCount/TagCount.tsx | 22 ++-- .../__snapshots__/TagCount.test.tsx.snap | 16 ++- .../UnavailableContent/UnavailableContent.tsx | 25 ++-- .../UnavailableContent.test.tsx.snap | 24 +++- .../module/src/WarningModal/WarningModal.tsx | 9 +- .../__snapshots__/WarningModal.test.tsx.snap | 8 +- 29 files changed, 387 insertions(+), 154 deletions(-) diff --git a/README.md b/README.md index 6e842c00..75e40109 100644 --- a/README.md +++ b/README.md @@ -128,6 +128,7 @@ When adding/making changes to a component, always make sure your code is tested: - use React Testing Library for testing - add tests to a `[ComponentName].test.tsx` file to your component's directory - make sure all the core logic is covered +- add `ouiaId` to component props definition with a default value of the component name (for subcomponents, let's use `ComponentName-element-specification` naming convention e.g. `ouiaId="WarningModal-confirm-button"`) ### Styling: - for styling always use JSS diff --git a/packages/module/src/Ansible/Ansible.tsx b/packages/module/src/Ansible/Ansible.tsx index cea29462..0d3a6565 100644 --- a/packages/module/src/Ansible/Ansible.tsx +++ b/packages/module/src/Ansible/Ansible.tsx @@ -5,8 +5,10 @@ import { createUseStyles } from 'react-jss'; export interface AnsibleProps extends React.DetailedHTMLProps, HTMLElement> { /** Supported/unsupported variant flag */ unsupported?: boolean | number; - /** Ansible icon className*/ + /** Ansible icon className */ className?: string; + /** Custom OUIA ID */ + ouiaId?: string | number; } const useStyles = createUseStyles({ @@ -30,7 +32,7 @@ const useStyles = createUseStyles({ } }) -const Ansible: React.FunctionComponent = ({ unsupported, className, ...props }: AnsibleProps) => { +const Ansible: React.FunctionComponent = ({ unsupported, className, ouiaId = "Ansible-icon", ...props }: AnsibleProps) => { const classes = useStyles(); const ansibleLogoClass = clsx( classes.ansible, @@ -71,7 +73,7 @@ const Ansible: React.FunctionComponent = ({ unsupported, className ); return ( - + = ({ severity, label, labelHidden, className, ...props }: BatteryProps) => { +const Battery: React.FunctionComponent = ({ severity, label, labelHidden, className, ouiaId = 'Battery-icon', ...props }: BatteryProps) => { const classes = useStyles(); const batteryClasses = clsx(classes.battery, classes[String(batteryLevels(severity, true))], className); @@ -103,7 +105,7 @@ const Battery: React.FunctionComponent = ({ severity, label, label return ( {/* eslint-disable-next-line react/no-unknown-property */} - + = ({ className, dataTestID, onClick, + ouiaId="CloseButton", ...props }: CloseButtonProps) => { const classes = useStyles(); @@ -30,6 +31,7 @@ const CloseButton: React.FunctionComponent = ({ data-test-id={dataTestID} onClick={onClick} variant={ButtonVariant.plain} + ouiaId={ouiaId} {...props} > diff --git a/packages/module/src/ErrorBoundary/ErrorBoundary.tsx b/packages/module/src/ErrorBoundary/ErrorBoundary.tsx index bd0c48f5..2e4bca63 100644 --- a/packages/module/src/ErrorBoundary/ErrorBoundary.tsx +++ b/packages/module/src/ErrorBoundary/ErrorBoundary.tsx @@ -18,6 +18,8 @@ export interface ErrorPageProps { defaultErrorDescription?: React.ReactNode; /** The component that the error boundary component is wrapped around, which should be returned if there is no error */ children?: React.ReactNode; + /** Custom OUIA ID */ + ouiaId?: string | number; } export interface ErrorPageState { @@ -61,33 +63,36 @@ class ErrorBoundary extends React.Component { } render() { + const { ouiaId = 'ErrorBoundary', ...props } = this.props; + if (this.state.hasError) { if (this.props.silent) { return null; } + return ( - - {this.props.headerTitle} +
+ {props.headerTitle} - {this.props.errorDescription} + {props.errorDescription} {this.state.error && ( - - + + )} } - defaultErrorDescription={this.props.defaultErrorDescription} + defaultErrorDescription={props.defaultErrorDescription} /> - +
); } - return this.props.children; + return props.children; } -} +}; export default ErrorBoundary; \ No newline at end of file diff --git a/packages/module/src/ErrorState/ErrorState.tsx b/packages/module/src/ErrorState/ErrorState.tsx index 2b93784b..d9cdfea9 100644 --- a/packages/module/src/ErrorState/ErrorState.tsx +++ b/packages/module/src/ErrorState/ErrorState.tsx @@ -29,27 +29,29 @@ export interface ErrorStateProps extends Omit { defaultErrorDescription?: React.ReactNode; /** Custom footer content */ customFooter?: React.ReactNode; + /** ErrorState OUIA ID */ + ouiaId?: string | number; } -const ErrorState: React.FunctionComponent = ({ errorTitle = 'Something went wrong', errorDescription, defaultErrorDescription, customFooter, ...props }: ErrorStateProps) => { +const ErrorState: React.FunctionComponent = ({ errorTitle = 'Something went wrong', errorDescription, defaultErrorDescription, customFooter, ouiaId = "ErrorState", ...props }: ErrorStateProps) => { const classes = useStyles(); return ( - {errorTitle}} icon={} headingLevel="h4" /> - + {errorTitle}} icon={} headingLevel="h4" data-ouia-component-id={`${ouiaId}-header`}/> + {errorDescription ? {errorDescription} : defaultErrorDescription} - + { customFooter || (document.referrer ? ( - ) : ( - ))} diff --git a/packages/module/src/InvalidObject/InvalidObject.tsx b/packages/module/src/InvalidObject/InvalidObject.tsx index 669f156a..1b9178e9 100644 --- a/packages/module/src/InvalidObject/InvalidObject.tsx +++ b/packages/module/src/InvalidObject/InvalidObject.tsx @@ -1,32 +1,35 @@ -import { Button, EmptyState, EmptyStateBody, EmptyStateFooter, EmptyStateHeader, EmptyStateVariant } from '@patternfly/react-core'; +import { Button, EmptyState, EmptyStateBody, EmptyStateFooter, EmptyStateHeader, EmptyStateProps, EmptyStateVariant } from '@patternfly/react-core'; import NotFoundIcon from '../NotFoundIcon/NotFoundIcon'; import React from 'react'; -export interface InvalidObjectProps extends React.DetailedHTMLProps, HTMLElement> { - /** The URL that the landing page link points to */ - toLandingPageUrl?: string; - /** The text label for the link that points back to the landing page */ - toLandingPageText?: React.ReactNode; +export interface InvalidObjectProps extends Omit { + /** The URL that the home page link points to */ + toHomePageUrl?: string; + /** The text label for the link that points back to the home page */ + toHomePageText?: React.ReactNode; /** The title for the invalid object message */ invalidObjectTitleText?: string; /** The body text for the invalid object message */ invalidObjectBodyText?: string; + /** Custom OUIA ID */ + ouiaId?: string | number; } - const InvalidObject: React.FunctionComponent = ({ - toLandingPageUrl = window.location.origin, - toLandingPageText = 'Return to homepage', + toHomePageUrl = window.location.origin, + toHomePageText = 'Return to homepage', invalidObjectTitleText = 'We lost that page', - invalidObjectBodyText = "Let's find you a new one. Try a new search or return home." + invalidObjectBodyText = "Let's find you a new one. Try a new search or return home.", + ouiaId = "InvalidObject", + ...props }: InvalidObjectProps) => ( - - } headingLevel='h1' /> - {invalidObjectBodyText} - - diff --git a/packages/module/src/InvalidObject/__snapshots__/InvalidObject.test.tsx.snap b/packages/module/src/InvalidObject/__snapshots__/InvalidObject.test.tsx.snap index 7325579d..a467d80b 100644 --- a/packages/module/src/InvalidObject/__snapshots__/InvalidObject.test.tsx.snap +++ b/packages/module/src/InvalidObject/__snapshots__/InvalidObject.test.tsx.snap @@ -7,16 +7,19 @@ exports[`InvalidObject component should render 1`] = `
Let's find you a new one. Try a new search or return home.
Let's find you a new one. Try a new search or return home.
= ({ logSnippet, message, leftBorderVariant = LogSnippetBorderVariant.danger, ...props }: LogSnippetProps) => { +export const LogSnippet: React.FunctionComponent = ({ logSnippet, message, leftBorderVariant = LogSnippetBorderVariant.danger, ouiaId = "LogSnippet", ...props }: LogSnippetProps) => { const classes = useStyles(leftBorderVariant); return ( - + - { typeof message === 'string' ? {message} : message } + { typeof message === 'string' ? {message} : message } { logSnippet && - - {logSnippet} + + {logSnippet} } diff --git a/packages/module/src/LogSnippet/__snapshots__/LogSnippet.test.tsx.snap b/packages/module/src/LogSnippet/__snapshots__/LogSnippet.test.tsx.snap index 7ba64f35..a977ad38 100644 --- a/packages/module/src/LogSnippet/__snapshots__/LogSnippet.test.tsx.snap +++ b/packages/module/src/LogSnippet/__snapshots__/LogSnippet.test.tsx.snap @@ -7,13 +7,14 @@ exports[`LogSnippet component should render LogSnippet component 1`] = ` 

                 
     

       

         

         

           

             
                = ({
   isExpandable = false,
   defaultExpanded = true,
   withHeaderBorder = false,
-  ...rest
+  ouiaId = 'MultiContentCard',
+  ...props
 }: MultiContentCardProps) => {
   const classes = useStyles(leftBorderVariant);
   const [ isExpanded, setIsExpanded ] = React.useState(defaultExpanded);
@@ -103,7 +106,7 @@ const MultiContentCard: React.FunctionComponent = ({
               inset={{ default: 'inset3xl' }}
             />
           )}
-          
+          
             {isCardWithProps(card) ? card.content : card}
           
           {(index + 1 < cards.length && (withDividers || isCardWithProps(card) && card.dividerVariant === MultiContentCardDividerVariant.right)) && (
@@ -118,9 +121,10 @@ const MultiContentCard: React.FunctionComponent = ({
   );
   
   return(
-    
+    
       {isExpandable && (
          = ({
           }}
           actions={{ actions }}
         >
-          {toggleText ? {toggleText} : toggleContent}
+          {toggleText ? {toggleText} : toggleContent}
         
       )}
-      {isExpandable ? {renderCards(cards, withDividers)} : renderCards(cards, withDividers)}
+      {isExpandable ? {renderCards(cards, withDividers)} : renderCards(cards, withDividers)}
     
   );}
 
diff --git a/packages/module/src/MultiContentCard/__snapshots__/MultiContentCard.test.tsx.snap b/packages/module/src/MultiContentCard/__snapshots__/MultiContentCard.test.tsx.snap
index c82ae923..feda964d 100644
--- a/packages/module/src/MultiContentCard/__snapshots__/MultiContentCard.test.tsx.snap
+++ b/packages/module/src/MultiContentCard/__snapshots__/MultiContentCard.test.tsx.snap
@@ -3,7 +3,7 @@
 exports[`MultiContentCard component should render basic multi content card 1`] = `
 

     

       

     

       

     

       

   

     

         

           

   

     

         

           

   

     

       

         

       

         

       

         

   

     

         

           

     

       

     

       

     

       

     

       

     

       

     

       
 = ({
@@ -39,20 +41,21 @@ const NotAuthorized: React.FunctionComponent = ({
   description = 'Contact your system administrator(s) for more information.',
   showReturnButton = true,
   className,
+  ouiaId = 'NotAuthorized',
   ...props
 }: NotAuthorizedProps) => (
-  
-    {title}} icon={} headingLevel="h5" />
-    {description}
-    
+  
+    {title}} icon={} headingLevel="h5" data-ouia-component-id={`${ouiaId}-header`} />
+    {description}
+    
       {primaryAction ? {primaryAction} : null}
       {showReturnButton && !primaryAction &&
             (document.referrer ? (
-              
             ) : (
-              
             ))}
diff --git a/packages/module/src/NotAuthorized/__snapshots__/NotAuthorized.test.tsx.snap b/packages/module/src/NotAuthorized/__snapshots__/NotAuthorized.test.tsx.snap
index 155fb1ac..3c796b70 100644
--- a/packages/module/src/NotAuthorized/__snapshots__/NotAuthorized.test.tsx.snap
+++ b/packages/module/src/NotAuthorized/__snapshots__/NotAuthorized.test.tsx.snap
@@ -3,12 +3,14 @@
 exports[`NotAuthorized component should apply custom styles 1`] = `