feat(Dropdowns): Clockface 4 dropdown updates #876
Conversation
ChitlangeSahas
changed the title
| margin-right: 2px; | ||
| margin-left: 1px; |
There was a problem hiding this comment.
These two stick out, but they're just to please the absolute positioned, Scrollbar library we're using
There was a problem hiding this comment.
Can this be solved using position and left and right rather than with using margins? Semantically, there's a big difference between position and margin, and we should use positioning for laying things out, and margins for when we need space around an element. I'm not super familiar with how this is used, though
There was a problem hiding this comment.
Got you! I'll try that!
There was a problem hiding this comment.
It still cuts off the right end of it, I think this is what is happening:
So, when I add the margin, the border falls under the available space.
Unfortunately, I am not able to change the width of the inner wrapper, since it's controlled by the react-custom-scrollbar library.
There was a problem hiding this comment.
This was always a problem, but we never noticed because we didn't use borders as an indicator for being selected. Now that we use the border, we see that the 1px of the right is cut off.
There was a problem hiding this comment.
is it causing problems to have the border? julia and i also discussed maybe not needing a background color at all on selected items. we included it because that's what we have now—adjusted to be lower contrast so it's less distracting—but i think the selected state for the radio buttons and checkboxes has enough contrast that we might not need the background color+border as well. let me know what you think. @ChitlangeSahas @hoorayimhelping @juliajames12
There was a problem hiding this comment.
Currently I implemented it in the way figma looks, with no issues. But if we decide to remove the border+background, let me know I can make a future ticket and address that! 👍
There was a problem hiding this comment.
talked to sahas—it's a big of a bigger project to reevaluate these, so we'll keep it as-is for now and tweak it later if we find we need to once we test it out
| @include buttonColorModifier( | ||
| $cf-turquoise, | ||
| $cf-turquoise, | ||
| $cf-grey-1, | ||
| $cf-grey-1 | ||
| ); |
There was a problem hiding this comment.
Just ran prettier, hence the diff.
ChitlangeSahas
requested review from
a team,
mavarius and
juliajames12
as code owners
| @@ -69,10 +72,18 @@ $dropdown-item--padding-v: $cf-space-2xs; | |||
| .cf-dropdown-divider { | |||
| @include dropdownItemStyles(); | |||
| font-weight: $cf-font-weight--medium; | |||
| text-transform: uppercase; | |||
| color: #88889b; | |||
There was a problem hiding this comment.
Since this is just a divider - does text-transform: uppercase have any effect?
There was a problem hiding this comment.
The dividers have text, optionally 👍
Closes #838
Couple screenshots:
SelectDropdown:TypeAheadDropdown:CreateableTypeAheadDropdown:MultiSelect: