Skip to content

[Design][Component] Feature: Add status circle buttons and sound playback button#3301

Open
wenjiangping wants to merge 3 commits into
goplus:uifrom
wenjiangping:issue-test001
Open

[Design][Component] Feature: Add status circle buttons and sound playback button#3301
wenjiangping wants to merge 3 commits into
goplus:uifrom
wenjiangping:issue-test001

Conversation

@wenjiangping

Copy link
Copy Markdown

Issue

  • Related to #

PR Type

  • Design PR
  • Demo PR (Draft)
  • Docs PR

Changes

  • Added Danger and Success circle button variants.
  • Added a button for sound playback.

Scope

In scope:

  • Visual styles for the Danger and Success circle buttons.
  • Icon and basic interaction states for the sound playback button.
  • Alignment with the existing Button component guidelines.

Validation

  • Checked the size, color, icon, and shape of the new buttons.
  • Checked visual consistency across button states.

Design Intent

  • Provide clear visual feedback for successful and destructive actions.
  • Introduce a reusable sound playback control for sound lists and previews.
  • Keep the new variants consistent with the existing component system.

Design Assets Changed

  • Button / Circle / Danger
  • Button / Circle / Success
  • Button / Sound Play

Production Handoff

  • Reuse the existing Button component and design tokens.
  • Implement Danger and Success as semantic variants instead of using hard-coded colors.
  • Support default, hover, pressed, and disabled states for the sound playback button.
  • Switch between play and pause feedback based on the current playback state.

@gemini-code-assist

Copy link
Copy Markdown
Contributor

Note

Gemini is unable to generate a review for this pull request due to the file types involved not being currently supported.

@fennoai fennoai Bot left a comment

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.

Code Review Summary

This PR adds sound playback buttons (Button/SoundPlay/36 and Button/SoundPlay/48) and circular status buttons (Button/Large/Danger/Circle/* and Button/Large/Success/Circle/*) to the component library. The overall structure is sound, but there are a few issues to address.

Findings:

  1. Hard-coded hex color instead of design token — The Stop/Default variant inside Button/SoundPlay/36 (and /48) uses raw hex #36C2CF for the stop icon fill, while every other new component uses tokens like $turquoise500. This should be replaced with the appropriate token to stay consistent with the rest of the library and to survive future palette changes.

  2. Inconsistent gap across Danger/Circle states — Patch 3 removed "gap": 10 from Button/Large/Danger/Circle/Default, but the five sibling states (Hover, Click, Loading, Disabled, Focus) still carry "gap": 10. All states of a component variant should be structurally identical; either remove gap from all states (if it serves no purpose for an icon-only circular button) or restore it on Default.

  3. Disabled placeholder layer in a published component — The frame ccr79 (icon/stop-filled) inside the Stop/Default state is marked "enabled": false. If this is a leftover debug/prototype layer it should be removed before merge. If it is intentional (e.g., a toggled-off alternative icon), add a note explaining the design intent so future maintainers don't treat it as dead weight.

  4. Missing Disabled/Focus states for sound-play buttonsButton/SoundPlay/36 and Button/SoundPlay/48 cover only Default/Hover/Click, while the new circular status buttons cover all 6 states. If sound-play buttons can be disabled or focused, these states are missing. If they are intentionally omitted (e.g., the button is never disabled), that is fine — just confirm the intent.

  5. Sub-pixel coordinate drift noise — Several diff hunks change coordinates by floating-point rounding noise (e.g., 14.99999999999999314.99999999999999, 5.682361148042482e-145.682361148043401e-14). These have no visual effect but inflate the diff and obscure meaningful changes. Consider snapping coordinates on export, or squash these into a separate cleanup commit so they don't hide real changes during review.

"y": 2.6666667461395264,
"name": "stop",
"geometry": "M0 13l0-10c0-2 1-3 3-3l10 0c2 0 3 1 3 3l0 10c0 2-1 3-3 3l-10 0c-2 0-3-1-3-3z",
"fill": "#36C2CF",

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.

Hard-coded hex #36C2CF here (and the same value appears in the 48px Stop/Default variant). All other fills in this PR use design tokens like $turquoise500. Replace with the appropriate token so the color stays in sync with palette changes.

"x": 10,
"y": 10,
"name": "icon/stop- filled",
"enabled": false,

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.

This "enabled": false child frame (icon/stop-filled) will be invisible in the rendered component. If it is a leftover prototype layer, remove it. If it is intentionally kept as a toggled-off variant, add a comment or note explaining why so future maintainers understand the intent.

{
"type": "frame",
"id": "y3vG5",
"name": "Button/Large/Danger/Circle/Default",

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.

Button/Large/Danger/Circle/Default has no gap after patch 3 removed it, but the Hover, Click, Loading, Disabled, and Focus siblings still carry "gap": 10. All states of a component variant should be structurally identical — either remove gap from all six states or restore it here.

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.

1 participant