Skip to content

Conversation

@toyamarinyon
Copy link
Contributor

Summary

Adds a destructive variant to the Button component for actions that require a clear warning.

Related Issue

N/A

Changes

  • Added "destructive" to the ButtonStyle type.
  • Implemented destructive variant styles using bg-error-900/10, text-error-900, border-error-900/20, and hover:bg-error-900/20.
  • Conditionally applied text-error-900 to button text and icons when the destructive style is active.

Testing

  • Verified formatting, type checks, and linting.
  • Confirmed visual styles for the new destructive variant.

Other Information

The destructive variant uses error-related colors, similar to the styling seen in revoke-invitation-dialog.tsx, to visually indicate a potentially irreversible action.


Open in Cursor Open in Web

@cursor
Copy link

cursor bot commented Nov 6, 2025

Cursor Agent can help with this pull request. Just @cursor in comments and I'll start working on changes in this branch.
Learn more about Cursor Agents

@changeset-bot
Copy link

changeset-bot bot commented Nov 6, 2025

⚠️ No Changeset found

Latest commit: bdc38ff

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

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

💥 An error occurred when fetching the changed packages and changesets in this PR
Some errors occurred when validating the changesets config:
The package or glob expression "giselles-ai" is specified in the `ignore` option but it is not found in the project. You may have misspelled the package name or provided an invalid glob expression. Note that glob expressions must be defined according to https://www.npmjs.com/package/micromatch.

@vercel
Copy link

vercel bot commented Nov 6, 2025

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

Project Deployment Preview Comments Updated (UTC)
giselle Ready Ready Preview Comment Nov 6, 2025 7:20am
ui Ready Ready Preview Comment Nov 6, 2025 7:20am

@giselles-ai
Copy link

giselles-ai bot commented Nov 6, 2025

Finished running flow.

Step 1
🟢
On Pull Request OpenedStatus: Success Updated: Nov 6, 2025 7:14am
Step 2
🟢
Manual QAStatus: Success Updated: Nov 6, 2025 7:16am
🟢
Prompt for AI AgentsStatus: Success Updated: Nov 6, 2025 7:16am
Step 3
🟢
Create a Comment for PRStatus: Success Updated: Nov 6, 2025 7:18am
Step 4
🟢
Create Pull Request CommentStatus: Success Updated: Nov 6, 2025 7:18am

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 6, 2025

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch cursor/add-destructive-variant-to-button-component-9a41

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@giselles-ai
Copy link

giselles-ai bot commented Nov 6, 2025

📋 Manual QA Checklist

Based on the changes in this PR, here are the key areas to test manually:

  • [Button Component - Destructive Variant]: Verify that a button with style="destructive" renders with a faint reddish background, red text, and a subtle red border.
  • [Button Component - Destructive Variant]: Hover over the destructive button and confirm its background becomes a slightly darker shade of red.
  • [Button Component - Destructive Variant]: Inspect the text within a destructive button to ensure it has the class text-error-900.
  • [Button Component - Destructive Variant]: For destructive buttons with leftIcon or rightIcon, inspect the icons to confirm they have the text-error-900 color.
  • [Button Component - Destructive Variant]: Click a functional destructive button to ensure its onClick event is triggered.
  • [Button Component - Existing Variants]: Visually inspect buttons with style="primary", style="glass", style="outline", and style="link" to ensure their appearance (background, border, text color, hover effects) remains unchanged.
  • [Button Component - Existing Variants]: For non-destructive buttons (e.g., style="primary") that include an icon, verify that the icon's color remains its default color and does not turn red.

✨ Prompt for AI Agents

Use the following prompts with Cursor or Claude Code to automate E2E testing:

📝 E2E Test Generation Prompt
You are an expert QA engineer. Your task is to generate a new E2E test file using Playwright for a React application. The test will validate the changes introduced in a recent Pull Request, which adds a "destructive" variant to a `Button` component.

Follow the instructions below to create a comprehensive, maintainable, and CI-ready test suite.

### 1. Context Summary

*   **PR Change:** The PR introduces a new `destructive` variant to the shared `Button` component.
*   **User Flow:** This button variant is intended for critical actions that are irreversible, such as "Delete Account" or "Revoke Invitation". When a developer uses `<Button style="destructive">`, it should render with a distinct, warning-like (red) appearance.
*   **Critical Paths to Test:**
    1.  Verify that a button with the `style="destructive"` prop correctly applies the new destructive CSS classes and styles.
    2.  Ensure the button text and any icons passed to it also adopt the destructive color scheme.
    3.  Confirm that this change does not negatively impact (regress) the styling of other existing button variants like `primary` or `outline`.

### 2. Test Scenarios

Create a test suite in a new file named `button-destructive.spec.ts`. This suite should cover the following scenarios:

**Happy Paths (Destructive Variant):**

1.  **Base Destructive Button:**
    *   Render a button with `style="destructive"`.
    *   Assert that it is visible.
    *   Assert that it has the correct `data-style="destructive"` attribute.
    *   Assert that it contains the core CSS classes for the destructive style.
2.  **Destructive Button with Text:**
    *   Render a destructive button with child text (e.g., "Delete").
    *   Assert that the inner `div` containing the text has the correct `text-error-900` class.
3.  **Destructive Button with Icons:**
    *   Render a destructive button with both a `leftIcon` and a `rightIcon`.
    *   Assert that the `div` wrappers for both icons receive the `*:text-error-900` class, which styles the icons correctly.
4.  **Hover State:**
    *   Render a destructive button.
    *   Simulate a mouse hover.
    *   Assert that the button's classes or computed styles change to reflect the hover state (e.g., presence of `hover:bg-error-900/20`).
5.  **Disabled Destructive Button:**
    *   Render a destructive button with the `disabled` attribute.
    *   Assert that it is disabled and not clickable.

**Regression Testing (Other Variants):**

1.  **Primary Button:**
    *   Render a standard button with `style="primary"`.
    *   Assert it has `data-style="primary"`.
    *   Assert that it **does not** contain any of the destructive-related CSS classes (`bg-error-900/10`, `text-error-900`, etc.).
2.  **Outline Button with Icon:**
    *   Render a button with `style="outline"` and an icon.
    *   Assert the icon's container **does not** have the `*:text-error-900` class.

### 3. Playwright Implementation Instructions

*   **Test File Setup:**
    *   Assume a component test setup where you can import and render the `Button` component directly on a test page. If not, navigate to a Storybook or a debug page where all button variants are displayed.
    *   For this exercise, let's assume you're writing a test that navigates to a test page like `/test-routes/buttons`.

*   **Selectors:**
    *   **Primary Selector:** Use `data-testid` attributes for robustness. If not available, use `page.getByRole('button', { name: 'Button Text' })`.
    *   **Variant Selector:** Use attribute selectors to target specific variants, e.g., `button[data-style="destructive"]`.

*   **User Interactions:**
    *   `await page.goto('/test-routes/buttons');` to navigate to the test page.
    *   `await buttonLocator.hover();` to test hover states.
    *   `await buttonLocator.click();` to verify interactivity (or lack thereof for disabled buttons).

*   **Assertions (Crucial):**
    *   Verify the presence of specific CSS classes introduced in the PR. This is the most reliable way to test styling changes without visual regression tools.
    *   Use `expect(buttonLocator).toHaveClass(/.../)`.

    ```typescript
    // Example: Asserting base destructive classes
    const destructiveButton = page.getByRole('button', { name: 'Delete' });
    await expect(destructiveButton).toHaveClass(/data-\[style=destructive]:bg-error-900\/10/);
    await expect(destructiveButton).toHaveClass(/data-\[style=destructive]:text-error-900/);

    // Example: Asserting text color class on the child element
    const buttonText = destructiveButton.locator('div').first();
    await expect(buttonText).toHaveClass(/text-error-900/);

    // Example: Asserting computed CSS for hover state
    await destructiveButton.hover();
    await expect(destructiveButton).toHaveCSS('background-color', 'rgba(127, 29, 29, 0.2)'); // Note: The exact RGB value might differ. Use the value your environment computes for `bg-error-900/20`.

    // Example: Regression check
    const primaryButton = page.getByRole('button', { name: 'Submit' });
    await expect(primaryButton).not.toHaveClass(/text-error-900/);
    ```

### 4. MCP Integration Guidelines (Optional)

*   **Execution Command:** The new test file should be runnable via the standard Playwright test runner. If a custom script `mcp` is used, the command would likely be:
    ```bash
    # Run the new specific test file
    mcp playwright:test --spec=tests/e2e/button-destructive.spec.ts

    # Or to run all tests
    mcp playwright:test
    ```
*   **Environment:** The tests should run against a local development server. Ensure the `playwright.config.ts` has the `webServer` property correctly configured to launch the dev environment.

### 5. CI-Ready Code Requirements

*   **Test Organization:**
    *   Wrap all tests for this feature in a `test.describe()` block for clarity and grouping.
    *   Use `test.beforeEach` to navigate to the test page to avoid repetition.
    ```typescript
    import { test, expect } from '@playwright/test';

    test.describe('Button Component: Destructive Variant', () => {
      test.beforeEach(async ({ page }) => {
        await page.goto('/test-routes/buttons');
      });

      // ... individual test cases go here
    });
    ```
*   **Naming Conventions:**
    *   **File:** `button-destructive.spec.ts`
    *   **Test Suite:** `test.describe('Button Component: Destructive Variant', ...)`
    *   **Test Cases:** Use clear, descriptive titles, e.g., `test('should apply destructive styles when style="destructive"', async ...)` or `test('should not affect the appearance of primary buttons', async ...)`.
*   **Error Handling & Assertions:** Use descriptive assertion messages where necessary, although Playwright's default messages are usually sufficient. For example: `expect(locator, 'The destructive button should be visible').toBeVisible()`.
*   **Parallelization:** These tests are self-contained and do not depend on each other. They are ideal for parallel execution. Ensure no state is shared between tests. The `test.describe.parallel()` directive can be used if desired.

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