feat(dashboards): add text widget handling in widget builder state functions

[nikkikapadia]

This PR is just dealing with all the widget builder state setting, retrieving and converting (from state to widget and vice versa). This is not really hooked up to anything within the widget builder yet but that's coming in my next PR.

Couple of things to note:

1. I've made the textContent state variable just a regular state instead of the query params state. We don't want to send large descriptions into the url params as we could hit url length limits and that would suck.
2. To compensate for ^that I've added in a session storage key which the initial data can be read from (this would be specifically for editing a widget and I will bake this into the editWidget functions in later PRs). The only downside to this is that when we copy and paste the url of the widget builder the text would not get copied with it.

I am open to other opinions on how to do this! Another thought I had was using a hash and compression function however sentry doesn't have one in the app already and would require further discussion with engineering peeps to see which library would be best for this. In my opinion that may be a little too much hassle for this relatively small feature. Nonetheless i'm all ears!

Closes DAIN-1320

[cursor]

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Autofix Details

Bugbot Autofix prepared a fix for the issue found in the latest run.

• ✅ Fixed: textContent leaks into query params via spread operator
  • I excluded textContent from the spread in convertBuilderStateToStateQueryParams and added a regression test asserting it is not present in query params.

Or push these changes by commenting:

@cursor push e5dfe4a601

Preview (e5dfe4a601)

diff --git a/static/app/views/dashboards/widgetBuilder/utils/convertBuilderStateToStateQueryParams.spec.tsx b/static/app/views/dashboards/widgetBuilder/utils/convertBuilderStateToStateQueryParams.spec.tsx
--- a/static/app/views/dashboards/widgetBuilder/utils/convertBuilderStateToStateQueryParams.spec.tsx
+++ b/static/app/views/dashboards/widgetBuilder/utils/convertBuilderStateToStateQueryParams.spec.tsx
@@ -56,6 +56,18 @@
     expect(queryParams.selectedAggregate).toBe(0);
   });
 
+  it('does not include text content in query params', () => {
+    const mockState: WidgetBuilderState = {
+      textContent: 'This should not be in URL params',
+      description: 'Description',
+    };
+
+    const queryParams = convertBuilderStateToStateQueryParams(mockState);
+
+    expect(queryParams).not.toHaveProperty('textContent');
+    expect(queryParams.description).toBe('Description');
+  });
+
   it('applies the thresholds to the query params', () => {
     const mockState: WidgetBuilderState = {
       query: ['transaction.duration:>100'],

diff --git a/static/app/views/dashboards/widgetBuilder/utils/convertBuilderStateToStateQueryParams.ts b/static/app/views/dashboards/widgetBuilder/utils/convertBuilderStateToStateQueryParams.ts
--- a/static/app/views/dashboards/widgetBuilder/utils/convertBuilderStateToStateQueryParams.ts
+++ b/static/app/views/dashboards/widgetBuilder/utils/convertBuilderStateToStateQueryParams.ts
@@ -11,7 +11,15 @@
 export function convertBuilderStateToStateQueryParams(
   state: WidgetBuilderState
 ): WidgetBuilderStateQueryParams {
-  const {fields, yAxis, sort, thresholds, traceMetric, ...rest} = state;
+  const {
+    fields,
+    yAxis,
+    sort,
+    thresholds,
+    traceMetric,
+    textContent: _textContent,
+    ...rest
+  } = state;
   return {
     ...rest,
     field: serializeFields(fields ?? []),

_{This Bugbot Autofix run was free. To enable autofix for future PRs, go to the Cursor dashboard.}

[linear-code]

DAIN-1320 Create field for text widget content in builder state

[gggritso]

Cool! Just a few nits, looks good overall

[gggritso]

👀 is it possible to use useLocalStorageState for this?

[cursor]

SET_STATE doesn't reset fields/sorts for text widget payload

Low Severity

The SET_STATE action for text widgets sets textContent and clears description, but doesn't explicitly reset data-oriented fields like fields, yAxis, sort, limit, thresholds, traceMetric, or axisRange when they're absent from the payload. The setFields and setYAxis calls are guarded by if (action.payload.field) / if (action.payload.yAxis), so if a text widget payload somehow omits those keys (rather than providing empty arrays), stale values from the previous widget type could persist. Current callers via convertWidgetToBuilderState always include field: [], so this works today, but the SET_DISPLAY_TYPE TEXT branch is more thorough in its cleanup by contrast.

 

[nikkikapadia]

usually the only times we use SET_STATE is for populating the state from caching and widget library. All of those have widget's that were already pre-defined a certain way so the same level of cleanup isn't necessary

[gggritso]

LGTM! I have one nit about function naming, but you can merge past it if you want 👍

[gggritso]

I appreciate this comment, but I'm still a bit confused about the distinction between convertWidgetToBuilderState and convertWidgetToBuilderStateParams. If I'm reading this right, it looks like the former is used to take a current widget configuration and convert it into the format in the builder state. The latter is for … turning a widget into a URL query? If that's right, would be worth updating the comments to explain the difference, and maybe even rename the latter function to convertWidgetToQueryParams or something similar to indicate what it's doing

[nikkikapadia]

ya good point it could be confusing, i'll rename and update description before merging 👍

[cursor]

Cursor Bugbot has reviewed your changes and found 1 potential issue.

There are 2 total unresolved issues (including 1 from previous review).

^{Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

[cursor]

Stores "undefined" string instead of removing session storage key

Low Severity

The third return value from useSessionStorage (_removeTextContent) is a proper removeItem function that deletes the key from session storage, but it's deliberately unused. Instead, setTextContent(undefined) is called in multiple places, which runs JSON.stringify(undefined) (returns JS undefined), causing sessionStorage.setItem to store the string "undefined". This only works because readStorageValue has a backward-compatibility check (value === 'undefined') with a comment explicitly stating "This should no longer happen." Using the provided remove function instead of setTextContent(undefined) would properly clean up the storage key.

Additional Locations (1)

• static/app/views/dashboards/widgetBuilder/hooks/useWidgetBuilderState.tsx#L445-L448

 

[nikkikapadia]

i don't want to remove the key until i close the widget builder