Skip to content

refactor(theme): use relative units for .pills#1871

Open
spliffone wants to merge 1 commit intomainfrom
feat/theme-pills-relative-sizing
Open

refactor(theme): use relative units for .pills#1871
spliffone wants to merge 1 commit intomainfrom
feat/theme-pills-relative-sizing

Conversation

@spliffone
Copy link
Copy Markdown
Member

@spliffone spliffone commented Apr 15, 2026

@spliffone spliffone requested review from a team as code owners April 15, 2026 05:29
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

The pull request updates the pill component by replacing pixel-based dimensions with rem units for block-size and border-radius. Feedback recommends using min-block-size instead of block-size to ensure the component scales with text and suggests increasing the border-radius to 1.5rem to preserve the capsule shape if the height expands.

Comment on lines +8 to +9
block-size: 1.5rem;
border-radius: 0.75rem;
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.

medium

Using min-block-size instead of block-size is recommended for components containing text. This ensures the pill can expand if the content wraps or if the font size is increased, preventing text overflow. Additionally, using a larger border-radius (e.g., 1.5rem) ensures the capsule shape is maintained even if the height increases. Note that for full consistency with the goal of using relative units, the padding (currently using pixel-based spacers) should also ideally be converted to rem.

Suggested change
block-size: 1.5rem;
border-radius: 0.75rem;
min-block-size: 1.5rem;
border-radius: 1.5rem;

Comment on lines +36 to +37
border-start-start-radius: 0.75rem;
border-end-start-radius: 0.75rem;
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.

medium

Using a larger value for the border radius ensures that the pill maintains its capsule shape even if its height increases due to content scaling.

Suggested change
border-start-start-radius: 0.75rem;
border-end-start-radius: 0.75rem;
border-start-start-radius: 1.5rem;
border-end-start-radius: 1.5rem;

Comment on lines +41 to +42
border-start-end-radius: 0.75rem;
border-end-end-radius: 0.75rem;
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.

medium

Using a larger value for the border radius ensures that the pill maintains its capsule shape even if its height increases due to content scaling.

Suggested change
border-start-end-radius: 0.75rem;
border-end-end-radius: 0.75rem;
border-start-end-radius: 1.5rem;
border-end-end-radius: 1.5rem;

@spliffone spliffone added the enhancement Topics that make the project better label Apr 15, 2026
@spliffone spliffone added this to the 49.x milestone Apr 15, 2026
align-items: center;
block-size: 24px;
border-radius: 12px;
block-size: 1.5rem;
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.

size should be natural. Looking at filtered-search where .pill is used, we have multiple problems there, e.g. forced line-height: 24px and the .pill block-size is used to make the pill smaller since the there's also padding-block

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

Labels

enhancement Topics that make the project better

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants