From 10f0e58c19c4a2fff722333c93f9bdd37305bd08 Mon Sep 17 00:00:00 2001 From: Ni Chia Date: Mon, 17 May 2021 10:27:57 -0400 Subject: [PATCH] [NO-TICKET] Fix non-string heading error (#1034) * Use ref to access heading content and fix max heading length of 100 * Add additional check for ref * Remove the extra variable check * Tidy Alert events tracking --- .../src/components/Alert/Alert.jsx | 28 +++++++++++++------ .../src/components/Dialog/Dialog.jsx | 23 +++++++++------ .../src/components/HelpDrawer/HelpDrawer.jsx | 22 ++++++++++----- .../src/components/analytics/SendAnalytics.ts | 2 ++ 4 files changed, 52 insertions(+), 23 deletions(-) diff --git a/packages/design-system/src/components/Alert/Alert.jsx b/packages/design-system/src/components/Alert/Alert.jsx index 9b34d08d4e..ac1146685b 100644 --- a/packages/design-system/src/components/Alert/Alert.jsx +++ b/packages/design-system/src/components/Alert/Alert.jsx @@ -1,7 +1,6 @@ -import { EVENT_CATEGORY, sendAnalyticsEvent } from '../analytics/SendAnalytics'; +import { EVENT_CATEGORY, MAX_LENGTH, sendAnalyticsEvent } from '../analytics/SendAnalytics'; import PropTypes from 'prop-types'; import React from 'react'; -import ReactDOMServer from 'react-dom/server'; import classNames from 'classnames'; import get from 'lodash/get'; import uniqueId from 'lodash.uniqueid'; @@ -22,12 +21,9 @@ const defaultAnalytics = (heading = '', variation = '') => ({ export class Alert extends React.PureComponent { constructor(props) { super(props); + this.alertRef = null; this.headingId = props.headingId || uniqueId('alert_'); - this.eventHeading = props.heading || props.children; - this.eventHeadingText = - typeof this.eventHeading === 'string' - ? this.eventHeading - : ReactDOMServer.renderToStaticMarkup(this.eventHeading); + this.eventHeadingText = ''; if (process.env.NODE_ENV !== 'production') { if (!props.heading && !props.children) { console.warn( @@ -39,12 +35,27 @@ export class Alert extends React.PureComponent { componentDidMount() { const eventAction = 'onComponentDidMount'; + const eventHeading = this.props.heading || this.props.children; + /* Send analytics event for `error`, `warn`, `success` alert variations */ - this.props.variation && + if (this.props.variation) { + if (typeof eventHeading === 'string') { + this.eventHeadingText = eventHeading.substring(0, MAX_LENGTH); + } else { + const eventHeadingTextElement = + (this.alertRef && this.alertRef.getElementsByClassName('ds-c-alert__heading')[0]) || + (this.alertRef && this.alertRef.getElementsByClassName('ds-c-alert__body')[0]); + this.eventHeadingText = + eventHeadingTextElement && eventHeadingTextElement.textContent + ? eventHeadingTextElement.textContent.substring(0, MAX_LENGTH) + : ''; + } + sendAnalyticsEvent( get(this.props.analytics, eventAction), get(defaultAnalytics(this.eventHeadingText, this.props.variation), eventAction) ); + } } heading() { @@ -71,6 +82,7 @@ export class Alert extends React.PureComponent { className={classes} role={this.props.role} aria-labelledby={this.props.heading ? this.headingId : undefined} + ref={(ref) => (this.alertRef = ref)} >
{this.heading()} diff --git a/packages/design-system/src/components/Dialog/Dialog.jsx b/packages/design-system/src/components/Dialog/Dialog.jsx index 1be6bc8d4f..bcab3d0126 100644 --- a/packages/design-system/src/components/Dialog/Dialog.jsx +++ b/packages/design-system/src/components/Dialog/Dialog.jsx @@ -1,9 +1,8 @@ -import { EVENT_CATEGORY, sendAnalyticsEvent } from '../analytics/SendAnalytics'; +import { EVENT_CATEGORY, MAX_LENGTH, sendAnalyticsEvent } from '../analytics/SendAnalytics'; import AriaModal from 'react-aria-modal'; import Button from '../Button/Button'; import PropTypes from 'prop-types'; import React from 'react'; -import ReactDOMServer from 'react-dom/server'; import classNames from 'classnames'; import get from 'lodash/get'; @@ -30,11 +29,8 @@ const defaultAnalytics = (heading = '') => ({ export class Dialog extends React.PureComponent { constructor(props) { super(props); - this.eventHeading = props.title || props.heading; - this.eventHeadingText = - typeof this.eventHeading === 'string' - ? this.eventHeading - : ReactDOMServer.renderToStaticMarkup(this.eventHeading); + this.headingRef = null; + this.eventHeadingText = ''; if (process.env.NODE_ENV !== 'production') { if (props.title) { @@ -57,6 +53,17 @@ export class Dialog extends React.PureComponent { componentDidMount() { const eventAction = 'onComponentDidMount'; + const eventHeading = this.props.title || this.props.heading; + + if (typeof eventHeading === 'string') { + this.eventHeadingText = eventHeading.substring(0, MAX_LENGTH); + } else { + this.eventHeadingText = + this.headingRef && this.headingRef.textContent + ? this.headingRef.textContent.substring(0, MAX_LENGTH) + : ''; + } + /* Send analytics event for dialog open */ sendAnalyticsEvent( get(this.props.analytics, eventAction), @@ -119,7 +126,7 @@ export class Dialog extends React.PureComponent { {...modalProps} >
-
+
(this.headingRef = ref)} className={headerClassNames} role="banner"> { // TODO: make heading required after removing title (title || heading) && ( diff --git a/packages/design-system/src/components/HelpDrawer/HelpDrawer.jsx b/packages/design-system/src/components/HelpDrawer/HelpDrawer.jsx index d4399133c2..41c33b5b30 100644 --- a/packages/design-system/src/components/HelpDrawer/HelpDrawer.jsx +++ b/packages/design-system/src/components/HelpDrawer/HelpDrawer.jsx @@ -1,8 +1,7 @@ -import { EVENT_CATEGORY, sendAnalyticsEvent } from '../analytics/SendAnalytics'; +import { EVENT_CATEGORY, MAX_LENGTH, sendAnalyticsEvent } from '../analytics/SendAnalytics'; import Button from '../Button/Button'; import PropTypes from 'prop-types'; import React from 'react'; -import ReactDOMServer from 'react-dom/server'; import classNames from 'classnames'; import get from 'lodash/get'; @@ -30,11 +29,8 @@ export class HelpDrawer extends React.PureComponent { constructor(props) { super(props); this.headingRef = null; - this.eventHeading = props.title || props.heading; - this.eventHeadingText = - typeof this.eventHeading === 'string' - ? this.eventHeading - : ReactDOMServer.renderToStaticMarkup(this.eventHeading); + this.eventHeadingText = ''; + if (process.env.NODE_ENV !== 'production') { if (props.title) { console.warn( @@ -51,7 +47,19 @@ export class HelpDrawer extends React.PureComponent { componentDidMount() { if (this.headingRef) this.headingRef.focus(); + const eventAction = 'onComponentDidMount'; + const eventHeading = this.props.title || this.props.heading; + + if (typeof eventHeading === 'string') { + this.eventHeadingText = eventHeading.substring(0, MAX_LENGTH); + } else { + this.eventHeadingText = + this.headingRef && this.headingRef.textContent + ? this.headingRef.textContent.substring(0, MAX_LENGTH) + : ''; + } + /* Send analytics event for helpdrawer open */ sendAnalyticsEvent( get(this.props.analytics, eventAction), diff --git a/packages/design-system/src/components/analytics/SendAnalytics.ts b/packages/design-system/src/components/analytics/SendAnalytics.ts index 596514bbbd..d2eb631243 100644 --- a/packages/design-system/src/components/analytics/SendAnalytics.ts +++ b/packages/design-system/src/components/analytics/SendAnalytics.ts @@ -36,6 +36,8 @@ export const EVENT_CATEGORY = { uiInteraction: 'ui interaction', }; +export const MAX_LENGTH = 100; + interface AnalyticsEventProps { ga_eventAction: string; ga_eventCategory: string;