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

Add variable for focused input background #1998

Merged
merged 5 commits into from
Oct 22, 2017

Conversation

aronstrandberg
Copy link
Contributor

Add a variable to make it possible to configure the input's background color when focused.

I was trying to theme the focused state in a project, but found it bit cumbersome, hence the desire for these changes. I'd be happy to make any requested changes from your side.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 91.994% when pulling a218808 on aronstrandberg:input-focus-bg into 1c2f096 on JedWatson:master.

@JedWatson
Copy link
Owner

Thanks @aronstrandberg, happy to accept this addition.

Would you please add it to the scss files as well though? we need to keep them in sync with the less, unfortunately this is a manual process.

Copy link
Owner

@JedWatson JedWatson left a comment

Choose a reason for hiding this comment

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

Please fix this up and add the .scss files as well, as we can merge this 👍

less/select.less Outdated
@@ -15,6 +15,7 @@
// control options
@select-input-bg: #fff;
@select-input-bg-disabled: #f9f9f9;
@select-input-bg-focus: #fff;
Copy link
Owner

Choose a reason for hiding this comment

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

Please make this default to @select-input-bg in case that has been customised, otherwise this would be a breaking change.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 91.994% when pulling ffe5b12 on aronstrandberg:input-focus-bg into 1c2f096 on JedWatson:master.

@aronstrandberg
Copy link
Contributor Author

@JedWatson Pushed some commits to adress these things. I couldn't find the equivalent of .Select-focus-state in the scss files, so I'm not 100%, but I think they should be similar now – is there a way to test the examples with the scss files to make sure?

@coveralls
Copy link

Coverage Status

Coverage remained the same at 91.994% when pulling 2fdedda on aronstrandberg:input-focus-bg into 1c2f096 on JedWatson:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 91.994% when pulling 2fdedda on aronstrandberg:input-focus-bg into 1c2f096 on JedWatson:master.

@aronstrandberg
Copy link
Contributor Author

@JedWatson I just rebased this on master to adjust for upstream changes. Do you think this is ready to merge?

@coveralls
Copy link

Coverage Status

Coverage remained the same at 92.344% when pulling 535bc23 on aronstrandberg:input-focus-bg into 7523c20 on JedWatson:master.

@aronstrandberg
Copy link
Contributor Author

@JedWatson?

@JedWatson
Copy link
Owner

Looks great @aronstrandberg, thanks!

@JedWatson JedWatson merged commit 3cf1b7d into JedWatson:master Oct 22, 2017
@aronstrandberg aronstrandberg deleted the input-focus-bg branch October 23, 2017 10:01
@abdennour
Copy link

Do not release feature with big bug :

Failed to compile.

Module build failed: 
		background: $select-input-bg-focus;
             ^
      Undefined variable: "$select-input-bg-focus".
      in /Users/abdennour/workspace/idax/node_modules/react-select/scss/control.scss (line 70, column 15)

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.

4 participants