Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update disabled Select style #337

Merged
merged 2 commits into from
Nov 22, 2017
Merged

Conversation

almgong
Copy link
Contributor

@almgong almgong commented Nov 22, 2017

The styles applied to disabled Select components did not match the appearance of other disabled inputs. This is because Select is built on top of react-select, which has its own defined styles. This PR updates the background color and cursor style for Select, in an effort to make the different disabled components look more consistent.

See: https://www.pivotaltracker.com/story/show/152485476

@almgong almgong changed the title Update disabled select style Update disabled Select style Nov 22, 2017
…ents

This is to match the disabled styles for other input types when the prop is passed to the component
@almgong almgong force-pushed the cldUpdateDisabledSelectStyle branch from 9e64f5a to 54ace57 Compare November 22, 2017 18:37
Copy link
Contributor

@gthomas-appfolio gthomas-appfolio 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, one small change requested

@@ -10,6 +10,7 @@ storiesOf('Select', module)
<div>
<Select
className="w-100"
disabled={boolean('Disabled', false)}
Copy link
Contributor

Choose a reason for hiding this comment

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

We have the knob name match the name/case of the prop it controls, so we should use "disabled"

@@ -4,7 +4,7 @@
$select-input-internal-height: 2.25rem !default;

// Override react-select styles to match Saffron (TODO move to theme?)
$select-input-bg-disabled: inherit;
$select-input-bg-disabled: #eceeef;
Copy link
Contributor

Choose a reason for hiding this comment

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

We try to avoid hard-coding colors, but I think it's fine in this case given the limitations of react-select and because #eceeef matches the saffron and MyCase themes.

@almgong almgong force-pushed the cldUpdateDisabledSelectStyle branch from 54ace57 to 36dfc9a Compare November 22, 2017 22:02
@gthomas-appfolio gthomas-appfolio merged commit f7870a9 into master Nov 22, 2017
@gthomas-appfolio gthomas-appfolio deleted the cldUpdateDisabledSelectStyle branch November 22, 2017 22:07
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.

2 participants