Skip to content

Commit

Permalink
[WNMGDS-1779] Do not show analytics content error when analytics are…
Browse files Browse the repository at this point in the history
… disabled (#1944)

Move error messaging up the call stack so it only fires when analytics enabled
  • Loading branch information
pwolfert authored Jul 6, 2022
1 parent 0e7ecf2 commit f78dffb
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 16 deletions.
11 changes: 8 additions & 3 deletions packages/design-system/src/components/Dialog/Dialog.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -327,7 +327,7 @@ export const Dialog = (props: DialogProps) => {
const escapeExitsProp = escapeExitDisabled ? !escapeExitDisabled : escapeExits;

function sendDialogEvent(
content: string,
content: string | undefined,
eventAttributes: { event_name: string; ga_eventAction: string }
) {
if (!dialogSendsAnalytics() || analytics === false) {
Expand All @@ -336,6 +336,11 @@ export const Dialog = (props: DialogProps) => {

const eventHeadingText = analyticsLabelOverride ?? content;

if (!eventHeadingText) {
console.error('No content found for Dialog analytics event');
return;
}

sendLinkEvent({
event_type: EventCategory.UI_INTERACTION,
ga_eventCategory: EventCategory.UI_COMPONENTS,
Expand All @@ -347,13 +352,13 @@ export const Dialog = (props: DialogProps) => {

const [headingRef] = useAnalyticsContent({
componentName: 'Dialog',
onMount: (content: string) => {
onMount: (content: string | undefined) => {
sendDialogEvent(content, {
event_name: 'modal_impression',
ga_eventAction: 'modal impression',
});
},
onUnmount: (content: string) => {
onUnmount: (content: string | undefined) => {
sendDialogEvent(content, {
event_name: 'modal_closed',
ga_eventAction: 'closed modal',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,7 @@ export function getAnalyticsContentFromRefs(
refs: RefObject<any>[],
componentName?: string
): string | undefined {
const content = refs.map((ref) => ref.current?.textContent).find((textContent) => textContent);
if (!content) {
console.error(`No content found for ${componentName ?? ''} analytics event`);
return;
}
return content;
return refs.map((ref) => ref.current?.textContent).find((textContent) => textContent);
}

export default getAnalyticsContentFromRefs;
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ export interface UseAnalyticsContentProps {
* Optional name of component for error messages
*/
componentName?: string;
onMount: (content: string) => any;
onUnmount?: (content: string) => any;
onMount: (content?: string) => any;
onUnmount?: (content?: string) => any;
}

/**
Expand All @@ -19,7 +19,11 @@ export interface UseAnalyticsContentProps {
* but will fall back to `bodyRef` (second ref) if no content is found:
*
* const [headingRef, bodyRef] = useAnalyticsContent({
* onMount: (content: string) => {
* onMount: (content: string | undefined) => {
* if (!content) {
* console.error('No content found for [component-name] analytics event');
* return;
* }
* sendLinkEvent({
* event_name: 'alert_impression',
* event_type: EventCategory.UI_INTERACTION,
Expand Down Expand Up @@ -58,10 +62,6 @@ export function useAnalyticsContent({
// onUnmount do not have a reason to change between renders.
useEffect(() => {
const content = getAnalyticsContentFromRefs(refs, componentName);
if (!content) {
return;
}

onMount(content);
return () => {
if (onUnmount) onUnmount(content);
Expand Down

0 comments on commit f78dffb

Please sign in to comment.