ref(feedback): Simplify feedback function params (#11957)

ryan953 · web-flow · commit 9f1975495a5b · 2024-05-08T13:48:42.000-07:00
We can simplify the function params and only pass options around most of the time. I also moved some dialog specific css variables into dialog.css.ts so they can be async loaded, which depends on the options config. Before submitting a pull request, please take a look at our [Contributing](https://github.com/getsentry/sentry-javascript/blob/master/CONTRIBUTING.md) guidelines and verify: - [ ] If you've added code that should be tested, please add tests. - [ ] Ensure your code lints and the test suite passes (`yarn lint`) & (`yarn test`).
diff --git a/packages/feedback/src/core/createMainStyles.ts b/packages/feedback/src/core/createMainStyles.ts
@@ -11,54 +11,26 @@ function getThemedCssVariables(theme: FeedbackInternalOptions['themeLight']): st
   --border: ${theme.border};
   --border-radius: ${theme.borderRadius};
   --box-shadow: ${theme.boxShadow};
-
-  --submit-background: ${theme.submitBackground};
-  --submit-background-hover: ${theme.submitBackgroundHover};
-  --submit-border: ${theme.submitBorder};
-  --submit-outline-focus: ${theme.submitOutlineFocus};
-  --submit-foreground: ${theme.submitForeground};
-  --submit-foreground-hover: ${theme.submitForegroundHover};
-
-  --cancel-background: ${theme.cancelBackground};
-  --cancel-background-hover: ${theme.cancelBackgroundHover};
-  --cancel-border: ${theme.cancelBorder};
-  --cancel-outline-focus: ${theme.cancelOutlineFocus};
-  --cancel-foreground: ${theme.cancelForeground};
-  --cancel-foreground-hover: ${theme.cancelForegroundHover};
-
-  --input-background: ${theme.inputBackground};
-  --input-foreground: ${theme.inputForeground};
-  --input-border: ${theme.inputBorder};
-  --input-outline-focus: ${theme.inputOutlineFocus};
-
-  --form-border-radius: ${theme.formBorderRadius};
-  --form-content-border-radius: ${theme.formContentBorderRadius};
   `;
 }
 
 /**
  * Creates <style> element for widget actor (button that opens the dialog)
  */
-export function createMainStyles(
-  colorScheme: 'system' | 'dark' | 'light',
-  themes: Pick<FeedbackInternalOptions, 'themeLight' | 'themeDark'>,
-): HTMLStyleElement {
+export function createMainStyles({ colorScheme, themeDark, themeLight }: FeedbackInternalOptions): HTMLStyleElement {
   const style = DOCUMENT.createElement('style');
   style.textContent = `
 :host {
-  --z-index: ${themes.themeLight.zIndex};
-  --font-family: ${themes.themeLight.fontFamily};
-  --font-size: ${themes.themeLight.fontSize};
+  --z-index: ${themeLight.zIndex};
+  --font-family: ${themeLight.fontFamily};
+  --font-size: ${themeLight.fontSize};
 
   font-family: var(--font-family);
   font-size: var(--font-size);
 
   --page-margin: 16px;
   --actor-inset: auto var(--page-margin) var(--page-margin) auto;
 
-  --dialog-inset: auto var(--page-margin) var(--page-margin) auto;
-  --dialog-padding: 24px;
-
   .brand-link path {
     fill: ${colorScheme === 'dark' ? '#fff' : '#362d59'};
   }
@@ -69,20 +41,21 @@ export function createMainStyles(
     }
   }
 
-  ${getThemedCssVariables(colorScheme === 'dark' ? themes.themeDark : themes.themeLight)}
+  ${getThemedCssVariables(colorScheme === 'dark' ? themeDark : themeLight)}
 }
 
 ${
   colorScheme === 'system'
     ? `
 @media (prefers-color-scheme: dark) {
   :host {
-    ${getThemedCssVariables(themes.themeDark)}
+    ${getThemedCssVariables(themeDark)}
   }
 }`
     : ''
 }
-}`;
+}
+`;
 
   return style;
 }
diff --git a/packages/feedback/src/core/integration.ts b/packages/feedback/src/core/integration.ts
@@ -84,7 +84,6 @@ export const buildFeedbackIntegration = ({
 
     // FeedbackTextConfiguration
     addScreenshotButtonLabel = ADD_SCREENSHOT_LABEL,
-    triggerLabel = TRIGGER_LABEL,
     cancelButtonLabel = CANCEL_BUTTON_LABEL,
     confirmButtonLabel = CONFIRM_BUTTON_LABEL,
     emailLabel = EMAIL_LABEL,
@@ -98,6 +97,7 @@ export const buildFeedbackIntegration = ({
     removeScreenshotButtonLabel = REMOVE_SCREENSHOT_LABEL,
     submitButtonLabel = SUBMIT_BUTTON_LABEL,
     successMessageText = SUCCESS_MESSAGE_TEXT,
+    triggerLabel = TRIGGER_LABEL,
 
     // FeedbackCallbacks
     onFormOpen,
@@ -163,7 +163,7 @@ export const buildFeedbackIntegration = ({
         DOCUMENT.body.appendChild(host);
 
         _shadow = host.attachShadow({ mode: 'open' });
-        _shadow.appendChild(createMainStyles(options.colorScheme, options));
+        _shadow.appendChild(createMainStyles(options));
       }
       return _shadow as ShadowRoot;
     };
diff --git a/packages/feedback/src/modal/components/Dialog.css.ts b/packages/feedback/src/modal/components/Dialog.css.ts
@@ -1,3 +1,4 @@
+import type { FeedbackInternalOptions } from '@sentry/types';
 import { DOCUMENT } from '../../constants';
 
 const DIALOG = `
@@ -237,19 +238,64 @@ const SUCCESS = `
 }
 `;
 
+function getThemedCssVariables(theme: FeedbackInternalOptions['themeLight']): string {
+  return `
+  --submit-background: ${theme.submitBackground};
+  --submit-background-hover: ${theme.submitBackgroundHover};
+  --submit-border: ${theme.submitBorder};
+  --submit-outline-focus: ${theme.submitOutlineFocus};
+  --submit-foreground: ${theme.submitForeground};
+  --submit-foreground-hover: ${theme.submitForegroundHover};
+
+  --cancel-background: ${theme.cancelBackground};
+  --cancel-background-hover: ${theme.cancelBackgroundHover};
+  --cancel-border: ${theme.cancelBorder};
+  --cancel-outline-focus: ${theme.cancelOutlineFocus};
+  --cancel-foreground: ${theme.cancelForeground};
+  --cancel-foreground-hover: ${theme.cancelForegroundHover};
+
+  --input-background: ${theme.inputBackground};
+  --input-foreground: ${theme.inputForeground};
+  --input-border: ${theme.inputBorder};
+  --input-outline-focus: ${theme.inputOutlineFocus};
+
+  --form-border-radius: ${theme.formBorderRadius};
+  --form-content-border-radius: ${theme.formContentBorderRadius};
+  `;
+}
+
 /**
  * Creates <style> element for widget dialog
  */
-export function createDialogStyles(): HTMLStyleElement {
+export function createDialogStyles({ colorScheme, themeDark, themeLight }: FeedbackInternalOptions): HTMLStyleElement {
   const style = DOCUMENT.createElement('style');
 
   style.textContent = `
-    ${DIALOG}
-    ${DIALOG_HEADER}
-    ${FORM}
-    ${BUTTON}
-    ${SUCCESS}
-  `;
+:host {
+  --dialog-inset: auto var(--page-margin) var(--page-margin) auto;
+  --dialog-padding: 24px;
+
+  ${getThemedCssVariables(colorScheme === 'dark' ? themeDark : themeLight)}
+}
+
+${
+  colorScheme === 'system'
+    ? `
+@media (prefers-color-scheme: dark) {
+  :host {
+    ${getThemedCssVariables(themeDark)}
+  }
+}`
+    : ''
+}
+}
+
+${DIALOG}
+${DIALOG_HEADER}
+${FORM}
+${BUTTON}
+${SUCCESS}
+`;
 
   return style;
 }
diff --git a/packages/feedback/src/modal/components/Dialog.tsx b/packages/feedback/src/modal/components/Dialog.tsx
@@ -1,4 +1,4 @@
-import type { FeedbackFormData } from '@sentry/types';
+import type { FeedbackFormData, FeedbackInternalOptions } from '@sentry/types';
 // biome-ignore lint/nursery/noUnusedImports: reason
 import { Fragment, h } from 'preact'; // eslint-disable-line @typescript-eslint/no-unused-vars
 import type { VNode } from 'preact';
@@ -11,12 +11,13 @@ import { Form } from './Form';
 import { SuccessIcon } from './SuccessIcon';
 
 interface Props extends HeaderProps, FormProps {
-  successMessageText: string;
   onFormSubmitted: () => void;
   open: boolean;
+  options: FeedbackInternalOptions;
 }
 
-export function Dialog({ open, onFormSubmitted, successMessageText, ...props }: Props): VNode {
+export function Dialog({ open, onFormSubmitted, ...props }: Props): VNode {
+  const options = props.options;
   const successIconHtml = useMemo(() => ({ __html: SuccessIcon().outerHTML }), []);
 
   const [timeoutId, setTimeoutId] = useState<NodeJS.Timeout | null>(null);
@@ -46,7 +47,7 @@ export function Dialog({ open, onFormSubmitted, successMessageText, ...props }:
     <Fragment>
       {timeoutId ? (
         <div class="success-message" onClick={handleOnSuccessClick}>
-          {successMessageText}
+          {options.successMessageText}
           <span class="success-icon" dangerouslySetInnerHTML={successIconHtml} />
         </div>
       ) : (
diff --git a/packages/feedback/src/modal/components/DialogHeader.tsx b/packages/feedback/src/modal/components/DialogHeader.tsx
@@ -6,17 +6,16 @@ import { useMemo } from 'preact/hooks';
 import { SentryLogo } from './SentryLogo';
 
 export interface Props {
-  formTitle: FeedbackInternalOptions['formTitle'];
-  showBranding: FeedbackInternalOptions['showBranding'];
+  options: FeedbackInternalOptions;
 }
 
-export function DialogHeader({ formTitle, showBranding }: Props): VNode {
+export function DialogHeader({ options }: Props): VNode {
   const logoHtml = useMemo(() => ({ __html: SentryLogo().outerHTML }), []);
 
   return (
     <h2 class="dialog__header">
-      {formTitle}
-      {showBranding ? (
+      {options.formTitle}
+      {options.showBranding ? (
         <a
           class="brand-link"
           target="_blank"
diff --git a/packages/feedback/src/modal/components/Form.tsx b/packages/feedback/src/modal/components/Form.tsx
@@ -13,25 +13,8 @@ import { FEEDBACK_WIDGET_SOURCE } from '../../constants';
 import { DEBUG_BUILD } from '../../util/debug-build';
 import { getMissingFields } from '../../util/validate';
 
-export interface Props
-  extends Pick<
-    FeedbackInternalOptions,
-    | 'addScreenshotButtonLabel'
-    | 'removeScreenshotButtonLabel'
-    | 'cancelButtonLabel'
-    | 'emailLabel'
-    | 'emailPlaceholder'
-    | 'isEmailRequired'
-    | 'isNameRequired'
-    | 'messageLabel'
-    | 'messagePlaceholder'
-    | 'nameLabel'
-    | 'namePlaceholder'
-    | 'showEmail'
-    | 'showName'
-    | 'submitButtonLabel'
-    | 'isRequiredLabel'
-  > {
+export interface Props extends Pick<FeedbackInternalOptions, 'showEmail' | 'showName'> {
+  options: FeedbackInternalOptions;
   defaultEmail: string;
   defaultName: string;
   onFormClose: () => void;
@@ -50,29 +33,33 @@ function retrieveStringValue(formData: FormData, key: string): string {
 }
 
 export function Form({
-  addScreenshotButtonLabel,
-  removeScreenshotButtonLabel,
-  cancelButtonLabel,
+  options,
   defaultEmail,
   defaultName,
-  emailLabel,
-  emailPlaceholder,
-  isEmailRequired,
-  isNameRequired,
-  messageLabel,
-  messagePlaceholder,
-  nameLabel,
-  namePlaceholder,
+
   onFormClose,
   onSubmit,
   onSubmitSuccess,
   onSubmitError,
   showEmail,
   showName,
-  submitButtonLabel,
-  isRequiredLabel,
   screenshotInput,
 }: Props): VNode {
+  const {
+    addScreenshotButtonLabel,
+    removeScreenshotButtonLabel,
+    cancelButtonLabel,
+    emailLabel,
+    emailPlaceholder,
+    isEmailRequired,
+    isNameRequired,
+    messageLabel,
+    messagePlaceholder,
+    nameLabel,
+    namePlaceholder,
+    submitButtonLabel,
+    isRequiredLabel,
+  } = options;
   // TODO: set a ref on the form, and whenever an input changes call proceessForm() and setError()
   const [error, setError] = useState<null | string>(null);
 
diff --git a/packages/feedback/src/modal/integration.tsx b/packages/feedback/src/modal/integration.tsx
@@ -16,7 +16,7 @@ export const feedbackModalIntegration = ((): FeedbackModalIntegration => {
       const user = getCurrentScope().getUser() || getIsolationScope().getUser() || getGlobalScope().getUser();
 
       const el = DOCUMENT.createElement('div');
-      const style = createDialogStyles();
+      const style = createDialogStyles(options);
 
       let originalOverflow = '';
       const dialog = {
@@ -50,27 +50,12 @@ export const feedbackModalIntegration = ((): FeedbackModalIntegration => {
       const renderContent = (open: boolean): void => {
         render(
           <Dialog
+            options={options}
             screenshotInput={screenshotInput}
-            showBranding={options.showBranding}
             showName={options.showName || options.isNameRequired}
             showEmail={options.showEmail || options.isEmailRequired}
-            isNameRequired={options.isNameRequired}
-            isEmailRequired={options.isEmailRequired}
-            formTitle={options.formTitle}
-            cancelButtonLabel={options.cancelButtonLabel}
-            submitButtonLabel={options.submitButtonLabel}
-            emailLabel={options.emailLabel}
-            emailPlaceholder={options.emailPlaceholder}
-            messageLabel={options.messageLabel}
-            messagePlaceholder={options.messagePlaceholder}
-            nameLabel={options.nameLabel}
-            namePlaceholder={options.namePlaceholder}
             defaultName={(userKey && user && user[userKey.name]) || ''}
             defaultEmail={(userKey && user && user[userKey.email]) || ''}
-            successMessageText={options.successMessageText}
-            isRequiredLabel={options.isRequiredLabel}
-            addScreenshotButtonLabel={options.addScreenshotButtonLabel}
-            removeScreenshotButtonLabel={options.removeScreenshotButtonLabel}
             onFormClose={() => {
               renderContent(false);
               options.onFormClose && options.onFormClose();
