Skip to content

fix(radio): [radio] resolve the issue of text wrapping not being centered in radio components #2884

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 11, 2025
Merged

fix(radio): [radio] resolve the issue of text wrapping not being centered in radio components #2884

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/vue/src/radio/src/token.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ export const classes = {
'mobile-icon-radio-disabled':
'[&_path:nth-of-type(2)]:fill-color-icon-disabled [&_path:nth-of-type(1)]:fill-color-bg-3 cursor-not-allowed',
'radio-input': 'absolute left-0 right-0 top-0 bottom-0 w-0 h-0 -z-10 opacity-0',
'radio-text-common': 'sm:h-4 text-center text-color-text-primary align-middle text-sm',
'radio-text-common': 'sm:h-4 text-color-text-primary align-middle text-sm',
Copy link

@petercat-assistant petercat-assistant bot Feb 11, 2025

Choose a reason for hiding this comment

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

The removal of 'text-center' from 'radio-text-common' class may affect the text alignment in other components using this class. Please ensure that this change does not introduce any unintended side effects in other parts of the application.

Copy link

@coderabbitai coderabbitai bot Feb 11, 2025

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Verify if removing text-center achieves the desired text wrapping behavior.

The PR aims to fix text wrapping not being centered, but removing the text-center class might have the opposite effect. The align-middle property only handles vertical alignment, not horizontal text alignment.

Could you please:

  1. Confirm if removing text-center is intentional
  2. Provide a screenshot or description of how this change improves the text wrapping behavior
  3. Consider if we need a different approach to center the wrapped text

🌐 Web query:

What's the difference between text-center and align-middle in CSS?

💡 Result:

In CSS, text-align: center and vertical-align: middle serve fundamentally different purposes:

Horizontal vs. Vertical Alignment

  1. text-align: center

    • Horizontally centers text or inline/inline-block elements within their container[1][6][8].
    • Works on block-level containers (<div>, <p>, etc.)[2][6].
    • Example: 
      .container { text-align: center; }

  2. vertical-align: middle

    • Vertically aligns inline/inline-block elements relative to their siblings/container[3][7][9].
    • Primarily used for:
      • Table cells (<td>, <th>)[3]
      • Inline elements (images/icons in text)[7]
    • Example: 
      img { vertical-align: middle; }

Key Differences

Property Axis Applies To Common Use Cases
text-align Horizontal Block containers Centering paragraphs/headings
vertical-align Vertical Inline/table-cell elements Aligning icons with text

Important Notes

  • vertical-align: middle ≠ full vertical centering of block elements - use Flexbox (align-items: center) or Grid for modern layouts[3][9].
  • text-align inherits by default, while vertical-align doesn’t[8].
  • Avoid deprecated HTML attributes like <center> or align="center" - use CSS instead[1][5].

Citations:

Review the horizontal centering of the radio text alignment

The removal of the text-center class, which applies horizontal centering via text-align: center, appears contradictory to the PR’s stated objective of fixing text wrapping alignment. The align-middle class only affects vertical alignment. Could you please confirm if removing text-center was intentional? If so, please provide additional context (e.g., a screenshot or description) showing how text wrapping is now correctly aligned. Otherwise, we might need to reconsider the approach to ensure horizontal centering remains effective.

'radio-text-size-common': 'sm:text-xs',
'radio-text-size-medium': 'sm:text-sm',
'radio-hover': 'sm:[&_path:nth-of-type(1)]:hover:fill-color-brand-hover',
Expand Down