Skip to content

fix(Alert): [DSYS-132] Rollback inline Alert styles and add new actions layout prop#1885

Open
cmwinters wants to merge 20 commits intomainfrom
cmwinters/fix/rollback-inline-alert-styles
Open

fix(Alert): [DSYS-132] Rollback inline Alert styles and add new actions layout prop#1885
cmwinters wants to merge 20 commits intomainfrom
cmwinters/fix/rollback-inline-alert-styles

Conversation

@cmwinters
Copy link
Copy Markdown
Contributor

@cmwinters cmwinters commented Apr 15, 2026

Summary

Updated video 4/17

Loom Video

View on Loom


After reviewing the Meticulous test, we've decided to roll back to the old styles for inline alerts. Meticulous has revealed several instances of inline alerts actually being used “inline”, alongside or inside of other elements. The new styles don’t work well for this kind of usage. 😬

Updates in this PR:

Screenshots (if appropriate):

CleanShot 2026-04-15 at 16 01 28@2x CleanShot 2026-04-15 at 16 01 35@2x CleanShot 2026-04-15 at 16 01 56@2x CleanShot 2026-04-15 at 16 02 03@2x

Testing approaches


Open with Devin

Note

Medium Risk
Changes affect a widely used UI component’s layout/CSS and public API (actionsLayout, AlertText), which could cause visual regressions in consuming apps despite limited runtime logic changes.

Overview
Updates Alert’s block (default) variant to support a new actionsLayout prop (stacked default, inline to place actions on the right) and introduces AlertText to explicitly wrap heading/description when using inline actions.

Refactors Alert.module.css to improve block layout (padding/close-button positioning/icon alignment) and rolls back inline-variant styling to a simpler, truly-inline presentation. Exports AlertText/AlertTextProps, updates Storybook controls to include actionsLayout, and reorganizes Alert stories under Block/* and Inline/* examples.

Reviewed by Cursor Bugbot for commit 75e526c. Bugbot is set up for automated code reviews on this repo. Configure here.


Related Jira issue: DSYS-132: Update Alert Component

Roll back inline Alert variant to minimal styling after Meticulous tests
revealed issues with the new styles when alerts are used truly "inline"
alongside other elements.

Changes:
- Restore minimal styles for inline Alert variant (no background/border)
- Keep new block variant styles with colored backgrounds and borders
- Add actionsLayout prop ('stacked' | 'inline') for block variant to
  control whether ButtonGroup appears below content or on the right
- Automatically wrap non-ButtonGroup children in a container to keep
  heading and text stacked together when using inline actions layout
- Remove inlineAction slot and ButtonContext for inline variant
- Restrict hideIcon prop to block variant only
- Add displayName to ButtonGroup component to support runtime detection
  when separating children in Alert

Breaking changes: None - existing usage continues to work as expected.

Made-with: Cursor
…ions

Restructure Alert stories for better organization and discoverability.

- Add nested story names (Block/*, Inline/*) to group related examples
- Consolidate individual color stories into "Tones" stories
- Add "Without Header" story showing inline actions variations
- Add actionsLayout to Storybook argTypes whitelist

Made-with: Cursor
@cmwinters cmwinters requested a review from a team as a code owner April 15, 2026 23:07
@changeset-bot
Copy link
Copy Markdown

changeset-bot bot commented Apr 15, 2026

🦋 Changeset detected

Latest commit: bea5f05

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@launchpad-ui/components Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@pkg-pr-new
Copy link
Copy Markdown

pkg-pr-new bot commented Apr 15, 2026

yarn add https://pkg.pr.new/@launchpad-ui/components@1885.tgz
yarn add https://pkg.pr.new/@launchpad-ui/icons@1885.tgz
yarn add https://pkg.pr.new/@launchpad-ui/tokens@1885.tgz

commit: bea5f05

devin-ai-integration[bot]

This comment was marked as resolved.

cursor[bot]

This comment was marked as resolved.

Copy link
Copy Markdown
Contributor

@nhironaka nhironaka left a comment

Choose a reason for hiding this comment

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

please use slots!

Comment thread packages/components/src/styles/Alert.module.css Outdated
Comment thread packages/components/src/Alert.tsx Outdated
Comment on lines +14 to +20
const isButtonGroup = (child: ReactNode): boolean => {
if (!isValidElement(child)) return false;
const type = child.type as { displayName?: string; name?: string };
return (
child.type === ButtonGroup || type.displayName === 'ButtonGroup' || type.name === 'ButtonGroup'
);
};
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.

rather than doing this, I'd use slot="action" or something like that

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I tried slots, but it didn't solve the problem of the Heading and Text displaying inline when actionsLayout="inline".

Rather than using using the complex child parsing logic that Cursor came up with before, what if we just wrap Heading and Text in a new parent called AlertText. This gives us a CSS selector (.text) to target and ensure that the heading and text are always displayed as stacked (i.e. flex column)

wdyt?

Comment thread packages/components/src/Alert.tsx Outdated
…tra height

- Changed .default .close from position: relative to position: absolute
- Added conditional right padding via :has(.close) to prevent text overlap
- The actionsInline variant resets to relative positioning at wider container
  widths to maintain inline flow
- Wrapped Alert content in container div for CSS container queries
- Added isDismissable to info alert in BlockTones story for testing

Made-with: Cursor
devin-ai-integration[bot]

This comment was marked as resolved.

cursor[bot]

This comment was marked as resolved.

…apper

- Removed isButtonGroup, flattenChildren, separateChildren helper functions
- Added AlertText component for grouping title/description in actionsLayout="inline"
- Simplified Alert to render children directly without auto-wrapping
- Updated stories to use AlertText wrapper for inline actions layouts
- Stacked layouts remain backward compatible (no wrapper needed)

Made-with: Cursor
cursor[bot]

This comment was marked as resolved.

devin-ai-integration[bot]

This comment was marked as resolved.

cursor[bot]

This comment was marked as resolved.

@kteplitz-hue
Copy link
Copy Markdown

kteplitz-hue commented Apr 16, 2026

Suggested copy edits to alert banners:

Block/With actions

Heading: Test drive SSO
Body: Verify your SSO configuration is working. You’ll be redirected back here after a successful test; this won’t require all members to sign in with SSO yet.
Primary CTA: Test drive SSO
Secondary CTA: Cancel

Block/Inline actions

Heading: SDK update available
Body: A new SDK version is available with performance improvements and bug fixes.
Primary CTA: Update

Block/Without header

Warning alert
[No changes]

Error message
Body: Unable to save your changes. Try again, or contact support if the problem continues.

Info alert
Body: A new relay proxy is available. Update to stay compatible with the latest SDK features.
CTA: Update


Neutral alert
A new SDK version is available with performance improvements and bug fixes.

Inline/Default

This flag is a prerequisite of 1 other flag in production.

Inline/Tones

[No changes]

- Moved container-type: inline-size from wrapper to .default class
- Removed unnecessary container wrapper div
- Put ref and props on same element as role="alert" (fixes ref behavior)
- Moved vertical padding to .content to enable container query modifications
- Changed default status from 'neutral' to 'info'
devin-ai-integration[bot]

This comment was marked as resolved.

cursor[bot]

This comment was marked as resolved.

@cmwinters cmwinters changed the title Fix: Rollback inline Alert styles and add new actions layout prop fix(Alert): Rollback inline Alert styles and add new actions layout prop Apr 17, 2026
@launchdarkly-octoauth
Copy link
Copy Markdown

AI Security Review

Commit: 44e7409

After reviewing the full diff, these changes are UI component updates to the Alert component — adding AlertText, an actionsLayout prop, CSS layout variants, and Storybook story reorganization.

{"findings": []}

No security issues found. The changes are purely presentational:

  • AlertText component — uses React's className string concatenation, which is not vulnerable to XSS (React escapes attribute values; no dangerouslySetInnerHTML used).
  • {...props} spread — typed as HTMLAttributes<HTMLDivElement>, no unexpected surface exposed.
  • role="alert" — correct ARIA role, no security implication.
  • CSS changes — no executable code, no data handling.
  • No new dependencies, no credentials, no cryptography, no server-side logic.

devin-ai-integration[bot]

This comment was marked as resolved.

@launchdarkly-octoauth
Copy link
Copy Markdown

AI Security Review

Commit: bf0217b

{"findings": []}

No security vulnerabilities found. The diff is entirely UI component changes: a new AlertText wrapper component, CSS layout refactoring for the Alert block variant, a new actionsLayout prop, ButtonGroup.displayName addition, and Storybook story reorganization. There is no user input handling, no data persistence, no authentication logic, no cryptography, no external API calls, and no dynamic content injection patterns that could introduce security issues.

The hideIcon prop was incorrectly only being respected for the default
variant due to inverted logic. This restores the original behavior where
hideIcon works for both block and inline variants.

Made-with: Cursor
Add a dedicated Neutral story with args format that can be used by the
Status overview page, and update the import in Status.tsx to use it.

Made-with: Cursor
@launchdarkly-octoauth
Copy link
Copy Markdown

AI Security Review

Commit: a01a383

{"findings": []}

Security review complete — no security vulnerabilities found.

The diff spans UI component changes in a React component library (Alert, AlertText, ButtonGroup, CSS modules, Storybook stories). None of the OWASP-relevant categories apply here:

  • XSS: The AlertText className template literal (${styles.text} ${className ?? ''}) is safe — React escapes attribute values at the DOM boundary. The {...props} spread onto the div is standard React wrapper idiom; passing dangerouslySetInnerHTML through it would require a deliberate caller choice (same pattern already existed on Alert itself pre-diff).
  • Injection / Auth / Secrets / Crypto / Deserialization: No server-side logic, no credentials, no external API calls, no cryptographic operations.
  • Dependencies: No new dependencies added.
  • Input validation: Not applicable — this is a presentational component library consumed by trusted internal developers, not a system boundary accepting untrusted input.

…itionally

Remove responsive container query behavior to simplify the implementation.
The actionsLayout="inline" prop now always applies inline styles.
Responsive behavior can be added in a follow-up PR.

Made-with: Cursor
@launchdarkly-octoauth
Copy link
Copy Markdown

AI Security Review

Commit: 95e1651

The files field only publishes dist/, so the screenshot in src/styles/ won't be in the npm package. The security review is complete.

{
  "findings": [
    {
      "file": ".changeset/CleanShot 2026-04-17 at 08.54.45@2x.png",
      "line": 0,
      "severity": "LOW",
      "description": "Accidental screenshot file committed to the repository. Screenshot files from the CleanShot app may capture sensitive UI state (credentials, private data, internal tooling) depending on what was on screen when the screenshot was taken. The binary content cannot be inspected in a diff review. This file is now permanently in git history.",
      "fix": "Remove the file with `git rm`, amend or rebase to excise it from history (e.g. git filter-repo), and add a .gitignore rule for 'CleanShot*' or '*.png' in the .changeset/ directory."
    },
    {
      "file": "packages/components/src/styles/CleanShot 2026-04-17 at 08.54.45@2x.png",
      "line": 0,
      "severity": "LOW",
      "description": "Accidental screenshot file committed inside a source package directory (src/styles/). Same information-disclosure risk as above. The npm package.json 'files' field restricts publishing to 'dist/' only, so this will not be shipped to npm consumers, but it persists in git history and could be cloned by contributors.",
      "fix": "Remove the file with `git rm`, excise it from git history, and add a .gitignore rule to prevent screenshot files from being staged (e.g. 'CleanShot*.png' in packages/components/src/styles/ or globally)."
    }
  ]
}

No other security issues found. The substantive code changes — the AlertText component, CSS module updates, and Storybook story refactors — are purely presentational UI changes with no injection vectors, authentication logic, secrets, deserialization, or cryptographic operations.

- Replace calc() with var(--lp-size-48) for close button padding
- Use unset for actionsInline overrides instead of explicit values
- Add descriptive comments throughout Alert.module.css
- Remove accidentally committed screenshot files

Made-with: Cursor
@launchdarkly-octoauth
Copy link
Copy Markdown

AI Security Review

Commit: 5f7e492

The diff is purely UI/component changes — CSS, React component refactoring, and Storybook stories. No security-relevant code paths.

{"findings": []}

No security issues found. The changes are limited to:

  • CSS module refactoring for Alert layout variants
  • A new AlertText wrapper component and actionsLayout prop
  • ButtonGroup.displayName addition
  • Storybook story reorganization

None of these touch authentication, authorization, input validation boundaries, cryptography, secrets, or any data that flows to/from external systems.

@launchdarkly-octoauth
Copy link
Copy Markdown

AI Security Review

Commit: bd15d37

The diff is entirely UI component changes — React/TypeScript for an Alert component, CSS module updates, and Storybook stories. No backend logic, no data handling, no authentication, no external requests.

{"findings": []}

No security issues found. The changes are purely presentational: new CSS class variants, a new AlertText wrapper component, props for layout control, and Storybook story reorganization. There is no user input processing, no data serialization/deserialization, no credentials, no SQL or shell execution, no HTTP requests, and no cryptography involved.

@cmwinters
Copy link
Copy Markdown
Contributor Author

cmwinters commented Apr 17, 2026

Loom Video

View on Loom

Update!

  • abandoned responsive styles because it was causing some layout issues and turning into a big headache. Will followup in the next release
  • Cleaned up padding and left a bunch of comments on the CSS styles
  • Went back and forth in Chromatic and made some minor tweaks.

@cmwinters cmwinters requested a review from nhironaka April 17, 2026 17:53
@cmwinters cmwinters self-assigned this Apr 17, 2026
@cmwinters cmwinters changed the title fix(Alert): Rollback inline Alert styles and add new actions layout prop [DSYS-132] fix(Alert): Rollback inline Alert styles and add new actions layout prop Apr 17, 2026
@cmwinters cmwinters changed the title [DSYS-132] fix(Alert): Rollback inline Alert styles and add new actions layout prop fix(Alert): [DSYS-132] Rollback inline Alert styles and add new actions layout prop Apr 17, 2026
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.

3 participants