Skip to content

fix(many): fix checkbox icons and fix border bugs in menu and in dateInput#2473

Merged
git-nandor merged 1 commit intomasterfrom
Fix_for_v12_rework
Mar 23, 2026
Merged

fix(many): fix checkbox icons and fix border bugs in menu and in dateInput#2473
git-nandor merged 1 commit intomasterfrom
Fix_for_v12_rework

Conversation

@git-nandor
Copy link
Contributor

@git-nandor git-nandor commented Mar 18, 2026

Use border-radius: inherit on Dialog, Menu, and Calendar so rounded
corners from ContextView flow through the component tree without
requiring overflow: clip (which was clipping CSS focus outlines).
Also broaden ToggleFacade icon selector from lucideIcon to icon.

Test:
check DateInput popup borders
check Menu borders
check Checkbox toggle variant icons

@git-nandor git-nandor changed the title fix(many): fix checkbox toggle variant icons and resolve border radiu… fix(many): fix checkbox icons and fix border bugs in menu and in dateInput Mar 18, 2026
@git-nandor git-nandor self-assigned this Mar 18, 2026
@github-actions
Copy link

github-actions bot commented Mar 18, 2026

PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-03-23 09:49 UTC

@balzss balzss requested review from HerrTopi and balzss March 18, 2026 13:32
@git-nandor git-nandor marked this pull request as ready for review March 18, 2026 13:33
@@ -169,6 +169,7 @@ class Dialog extends Component<DialogProps> {
role={role}
aria-label={this.props.label}
className={this.props.className}
Copy link
Collaborator

Choose a reason for hiding this comment

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

bit off, but I was really wondering why we allow className here. I think its a bug, since its there since the component was created, lets remove it c88cf8e#diff-3ca2f171e2d8a67947b8794d172024b87fb9de4fc3f0ead84fb15bd31fce56ccR212

Copy link
Contributor

Choose a reason for hiding this comment

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

might be because it's a util component, not a regular one. maybe we use that prop internally? i'd double check before removing it

Copy link
Contributor

Choose a reason for hiding this comment

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

i'd say let's add a todo comment and leave it as is, something like TODO: investigate if className is actually needed here or can be removed

Copy link
Collaborator

Choose a reason for hiding this comment

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

I could not find any usges of it, but someone else might be using it. Let's add a TODO for now, and remove it in v2

Copy link
Collaborator

@matyasf matyasf left a comment

Choose a reason for hiding this comment

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

Looks good, I could not find any visual issues

@@ -55,7 +55,7 @@ const generateStyle = (
padding: '0.25rem 0',
// TODO-rework
//background: componentTheme.background,
Copy link
Contributor

Choose a reason for hiding this comment

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

unrelated to the pr but i'm not sure why this is commented out

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i'll double check it with designer

Copy link
Contributor

Choose a reason for hiding this comment

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

if there is no straightworfard answer, let's leave it for now since it's out of the scope of this task anyway

Copy link
Collaborator

@matyasf matyasf left a comment

Choose a reason for hiding this comment

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

Looks good, please add a TODO to the strange className prop

…s inheritance issues in DateInput and Menu from View
@git-nandor git-nandor merged commit ab2c8c2 into master Mar 23, 2026
10 of 12 checks passed
@git-nandor git-nandor deleted the Fix_for_v12_rework branch March 23, 2026 09:49
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