Skip to content

Improve Search Attribute deletion messaging#3629

Open
laurakwhit wants to merge 1 commit into
mainfrom
DT-4190-delete-search-attributes-improvement
Open

Improve Search Attribute deletion messaging#3629
laurakwhit wants to merge 1 commit into
mainfrom
DT-4190-delete-search-attributes-improvement

Conversation

@laurakwhit

Copy link
Copy Markdown
Collaborator

Description & motivation 💭

  • Added an isExisting prop when adding a row so button to remove row is a close icon instead of a trash icon for new attributes.
  • In Cloud the trash button is disabled and shows a tooltip.
  • The getSupportedTypes prop was removed and the content component now defines the type list internally.
  • Added an optional description snippet prop with a fallback <p> that now includes a "Search Attribute docs" Link.
  • hideCancelButton prop removed since it was not being used.

Screenshots (if applicable) 📸

Default Cloud Cloud w/ delete enabled
Screenshot 2026-07-02 at 3 52 40 PM Screenshot 2026-07-02 at 3 51 16 PM Screenshot 2026-07-02 at 3 50 41 PM

Design Considerations 🎨

Testing 🧪

How was this tested 👻

  • Manual testing
  • E2E tests added
  • Unit tests added

Steps for others to test: 🚶🏽‍♂️🚶🏽‍♀️

For an existing search attribute

  • type select should be disabled
  • if the feature flag is enabled there should be a disabled delete button
  • if the feature flag not enabled there should be no delete button

For a new search attbibute

  • type select should be enabled
  • there should be a close button

Checklists

Draft Checklist

Merge Checklist

Issue(s) closed

DT-4190

Docs

Any docs updates needed?

@laurakwhit laurakwhit requested a review from a team as a code owner July 2, 2026 23:43
@vercel

vercel Bot commented Jul 2, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
holocene Ready Ready Preview, Comment Jul 2, 2026 11:43pm

Request Review


{#each $formData.attributes as _, index}
{#each $formData.attributes as _, index (index)}
<SearchAttributeRow

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • ⚠️ Type 'string | undefined' is not assignable to type 'string'.

{#each $formData.attributes as _, index}
{#each $formData.attributes as _, index (index)}
<SearchAttributeRow
name={$formData.attributes[index].name}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • ⚠️ Type 'string | undefined' is not assignable to type 'string'.

@@ -164,7 +207,9 @@
submitting={$submitting}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • ⚠️ Element implicitly has an 'any' type because index expression is not of type 'number'.

@temporal-cicd

temporal-cicd Bot commented Jul 2, 2026

Copy link
Copy Markdown
Contributor
Warnings
⚠️

📊 Strict Mode: 4 errors in 1 file (0.5% of 888 total)

src/lib/components/search-attributes-form/search-attributes-form-content.svelte (4)
  • L133:16: 'error' is of type 'unknown'.
  • L202:12: Type 'string | undefined' is not assignable to type 'string'.
  • L203:12: Type 'string | undefined' is not assignable to type 'string'.
  • L207:50: Element implicitly has an 'any' type because index expression is not of type 'number'.

Generated by 🚫 dangerJS against 2b37991

@bilal-karim

Copy link
Copy Markdown
Member
  • Can we remove the book-sparkles icon from the docs links?
  • In Default / Cloud with delete enabled, the inputs aren't aligned with the Type column heading.
  • Is there a reason we're showing "search attributes cannot be deleted" in the card description rather than just the delete button tooltip? Displaying it in the description when no attributes exist may be unnecessary. Could we only show it when search attributes >= 1? Or surface it as an informational alert banner instead of card description text?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants