Skip to content

fix(theme): make .btn-close always round#2075

Open
dr-itz wants to merge 2 commits into
mainfrom
fix/theme/btn-close-circle
Open

fix(theme): make .btn-close always round#2075
dr-itz wants to merge 2 commits into
mainfrom
fix/theme/btn-close-circle

Conversation

@dr-itz
Copy link
Copy Markdown
Member

@dr-itz dr-itz commented May 18, 2026

.btn-close is for compatibility with older markup for dialog close buttons that are supposed to be round.


Documentation.
Examples.
Dashboards Demo.
Playwright report.

Coverage Reports:

Code Coverage

@dr-itz dr-itz requested review from a team as code owners May 18, 2026 15:21
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 theme styles to make .btn-close circular by default and removes redundant button examples and snapshot entries. The review feedback points out that .btn-close might appear oval if used without the .btn class because square dimensions are not explicitly defined for it. Additionally, it is recommended to retain the removed examples to visually verify the new styling and ensure the buttons remain circular without the .btn-circle class.

I am having trouble creating individual review comments. Click here to see my feedback.

projects/element-theme/src/styles/bootstrap/_buttons.scss (217)

medium

While adding .btn-close here ensures a circular border radius, it will only appear perfectly round if the button is square. Currently, the styles that enforce a 1:1 aspect ratio and consistent block size are defined in a selector that requires the .btn class (line 202). If .btn-close is intended to be used standalone for compatibility with older markup (as suggested by its inclusion in the base styles at line 18 and the PR description), it may appear as an oval unless the square dimensions are also applied to it without the .btn prefix.

src/app/examples/buttons/buttons.html (141-148)

medium

Removing these examples eliminates the visual verification that .btn-close is now round by default without needing the .btn-circle class. The remaining examples in the "Circle buttons" section (lines 228-240) still use the .btn-circle class, which makes the new CSS for .btn-close redundant in those cases. Consider keeping these examples (perhaps moving them to the "Circle buttons" section) and removing the .btn-circle class from them to demonstrate and test the "always round" behavior of .btn-close itself.

@dr-itz dr-itz marked this pull request as draft May 18, 2026 15:28
@dr-itz dr-itz force-pushed the fix/theme/btn-close-circle branch 3 times, most recently from dc3ea2a to 2f5aa32 Compare May 18, 2026 15:44
@dr-itz dr-itz marked this pull request as ready for review May 18, 2026 15:58
@dr-itz dr-itz requested a review from spliffone May 18, 2026 16:37
@kfenner kfenner requested a review from chintankavathia as a code owner May 20, 2026 17:00
@dr-itz dr-itz force-pushed the fix/theme/btn-close-circle branch 3 times, most recently from e053d2a to 1abf142 Compare May 20, 2026 19:14
.btn-close is for compatibility with older markup for dialog close buttons
that are supposed to be round. Also fix the case where .btn-close needs
.btn which isn't supposed to be the case.
@dr-itz dr-itz force-pushed the fix/theme/btn-close-circle branch from 1abf142 to 4a0dcbd Compare May 21, 2026 12:06
@spliffone spliffone added this pull request to the merge queue May 27, 2026
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks May 27, 2026
Copy link
Copy Markdown
Member

@chintankavathia chintankavathia left a comment

Choose a reason for hiding this comment

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

👍

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