Skip to content

Commit

Permalink
[NO-TICKET] Fix non-string heading error (#1034)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
nichia authored May 17, 2021
1 parent 3e423f6 commit 10f0e58
Show file tree
Hide file tree
Showing 4 changed files with 52 additions and 23 deletions.
28 changes: 20 additions & 8 deletions packages/design-system/src/components/Alert/Alert.jsx
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -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(
Expand All @@ -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() {
Expand All @@ -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)}
>
<div className="ds-c-alert__body">
{this.heading()}
Expand Down
23 changes: 15 additions & 8 deletions packages/design-system/src/components/Dialog/Dialog.jsx
Original file line number Diff line number Diff line change
@@ -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';

Expand All @@ -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) {
Expand All @@ -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),
Expand Down Expand Up @@ -119,7 +126,7 @@ export class Dialog extends React.PureComponent {
{...modalProps}
>
<div role="document">
<header className={headerClassNames} role="banner">
<header ref={(ref) => (this.headingRef = ref)} className={headerClassNames} role="banner">
{
// TODO: make heading required after removing title
(title || heading) && (
Expand Down
22 changes: 15 additions & 7 deletions packages/design-system/src/components/HelpDrawer/HelpDrawer.jsx
Original file line number Diff line number Diff line change
@@ -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';

Expand Down Expand Up @@ -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(
Expand All @@ -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),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ export const EVENT_CATEGORY = {
uiInteraction: 'ui interaction',
};

export const MAX_LENGTH = 100;

interface AnalyticsEventProps {
ga_eventAction: string;
ga_eventCategory: string;
Expand Down

0 comments on commit 10f0e58

Please sign in to comment.