fix(many): fix checkbox icons and fix border bugs in menu and in dateInput#2473
fix(many): fix checkbox icons and fix border bugs in menu and in dateInput#2473git-nandor merged 1 commit intomasterfrom
Conversation
|
| @@ -169,6 +169,7 @@ class Dialog extends Component<DialogProps> { | |||
| role={role} | |||
| aria-label={this.props.label} | |||
| className={this.props.className} | |||
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
matyasf
left a comment
There was a problem hiding this comment.
Looks good, I could not find any visual issues
| @@ -55,7 +55,7 @@ const generateStyle = ( | |||
| padding: '0.25rem 0', | |||
| // TODO-rework | |||
| //background: componentTheme.background, | |||
There was a problem hiding this comment.
unrelated to the pr but i'm not sure why this is commented out
There was a problem hiding this comment.
i'll double check it with designer
There was a problem hiding this comment.
if there is no straightworfard answer, let's leave it for now since it's out of the scope of this task anyway
matyasf
left a comment
There was a problem hiding this comment.
Looks good, please add a TODO to the strange className prop
…s inheritance issues in DateInput and Menu from View
3c35198 to
b7392d4
Compare
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