Skip to content

refactor(theme): use relative units for .dropdown-item#1873

Open
spliffone wants to merge 1 commit intomainfrom
feat/dropdown-item-relative-sizing
Open

refactor(theme): use relative units for .dropdown-item#1873
spliffone wants to merge 1 commit intomainfrom
feat/dropdown-item-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 09:32
@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
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

This pull request updates the dropdown item styling by changing block-size to min-block-size to ensure a minimum height while allowing for content expansion. Feedback was provided to use relative units (rem) instead of absolute pixels (px) for better accessibility and alignment with the project's goals.

flex-wrap: nowrap;
align-items: center;
block-size: 32px;
min-block-size: 32px;
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

The pull request title specifies the use of relative units, but the implementation still uses an absolute pixel value (32px). To align with the PR's objective and improve accessibility (supporting browser font scaling), consider using rem units. 32px is equivalent to 2rem assuming a 16px base.

  min-block-size: 2rem;

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.

block-size should either be natural (i.e. given by content) or a calc with font-size/line-height + padding

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@dr-itz Usually I do agree but this means we need to adjust various places:

  • tabs
  • select
  • navbar
  • menu
  • typeahead
  • application-header
  • ...

Since the child elements sometimes contain text, text + icon, text + icon + badge, ... they all have different sizes. Before we simply set a fixed size so the padding was ignored and different combination resulted in the same size. Do you have an idea how to fix this issue efficiently?

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.

see #1880

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