Skip to content

refactor(theme): use relative units for dropdowns#1880

Open
dr-itz wants to merge 1 commit intomainfrom
refactor/theme/dropdown-rfs
Open

refactor(theme): use relative units for dropdowns#1880
dr-itz wants to merge 1 commit intomainfrom
refactor/theme/dropdown-rfs

Conversation

@dr-itz
Copy link
Copy Markdown
Contributor

@dr-itz dr-itz commented 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 introduces a new menu example and updates dropdown styles to use dynamic sizing and the :is() selector. Feedback focuses on restoring the !default flag to a theme variable to allow for overrides, removing unsupported text content from a menu divider, and adhering to the style guide by importing standalone components instead of an NgModule.

Comment thread projects/element-theme/src/styles/bootstrap/_variables.scss
Comment thread src/app/examples/si-menu/si-menu-plain.html Outdated
Comment thread src/app/examples/si-menu/si-menu-inline.ts
@dr-itz dr-itz force-pushed the refactor/theme/dropdown-rfs branch 2 times, most recently from a418e86 to bd1eb15 Compare April 15, 2026 14:16
@dr-itz dr-itz requested a review from spliffone April 15, 2026 14:36
@dr-itz dr-itz marked this pull request as ready for review April 15, 2026 14:37
@dr-itz dr-itz requested review from a team as code owners April 15, 2026 14:37
Copy link
Copy Markdown
Member

@spliffone spliffone left a comment

Choose a reason for hiding this comment

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

With the current approach the si-select template is limited to one line, do we have the changes to fix this behaviour too?

@dr-itz dr-itz force-pushed the refactor/theme/dropdown-rfs branch from bd1eb15 to 639a444 Compare April 16, 2026 11:19
@dr-itz
Copy link
Copy Markdown
Contributor Author

dr-itz commented Apr 16, 2026

With the current approach the si-select template is limited to one line, do we have the changes to fix this behaviour too?

done

@dr-itz dr-itz force-pushed the refactor/theme/dropdown-rfs branch from 639a444 to e0e8884 Compare April 16, 2026 13:05
@dr-itz
Copy link
Copy Markdown
Contributor Author

dr-itz commented Apr 16, 2026

With the current approach the si-select template is limited to one line, do we have the changes to fix this behaviour too?

done

nope, too much is broken

@dr-itz dr-itz force-pushed the refactor/theme/dropdown-rfs branch from e0e8884 to ca96bde Compare April 16, 2026 13:52
@dr-itz
Copy link
Copy Markdown
Contributor Author

dr-itz commented Apr 16, 2026

With the current approach the si-select template is limited to one line, do we have the changes to fix this behaviour too?

done

nope, too much is broken

reverted, too much for this PR and out of scope anyway

Comment thread src/app/examples/si-menu/si-menu-inline.html
Comment thread src/app/examples/si-menu/si-menu-inline.html
@dr-itz dr-itz force-pushed the refactor/theme/dropdown-rfs branch from ca96bde to 3c60720 Compare April 16, 2026 14:41
@dr-itz dr-itz requested a review from spike-rabbit April 16, 2026 19:12
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