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

Feature Request: Styling with styled-components #2146

Closed
tobilen opened this issue Nov 14, 2017 · 5 comments
Closed

Feature Request: Styling with styled-components #2146

tobilen opened this issue Nov 14, 2017 · 5 comments

Comments

@tobilen
Copy link

tobilen commented Nov 14, 2017

We are currently in the process of migrating our component library to styled-components, and react-select is kind of a showstopper there. The styling is complicated, and overriding it would be a bug-riddled mess. So for now, the plan is to keep our sass loader for it.

While react-select is a really great component, styling feels unwieldy and cumbersome. I'd propose porting it to styled-components while ditching all (now) unnecessary nesting in the process. All sc-components should of course be exportable for easy extending, plus a theme would be nice to cover the setting-sass-variables way of doing styling.

@agirton
Copy link
Collaborator

agirton commented Nov 15, 2017

Hi @tobilen we have discussed this in a few other issues #109, #1324, #1679. For 1.0 we will not be able to provide this, but will consider it for the next major release of the package.

@tobilen
Copy link
Author

tobilen commented Nov 15, 2017

hey @agirton , does that mean you would be open to accepting a PR for this?

@chenasraf
Copy link

What exactly would this require in order to implement?

@tobilen
Copy link
Author

tobilen commented Nov 19, 2017

sorry for the delay, i missed the notification

styled-components provides sass-like syntax for nested classes, so an mvp could just provide a couple of sc-component that act as an anchor point for the styles.

Simplified Example for control.scss:

import styled from 'styled-components';
import { boxSizing } from './helper'; // this file has to be created as well

const StyledSelect = styled.div`
        position: relative;

	// disable some browser-specific behaviours that break the input
	input::-webkit-contacts-auto-fill-button,
	input::-webkit-credentials-auto-fill-button {
		display: none !important;
	}

	// preferred box model
	&,
	& div,
	& input,
	& span {
                ${boxSizing('border-box')}
	}
`;

export default StyledSelect;

To-Do's:

Optional (but recommended) steps would be:

Subsequent PR's could then expand the styled-components idea and get rid of the nested classes, and instead introduce new sc-components that properly encapsulate styles. Also, wrapping the (previously created) style settings file in a styled-components theme.

@JedWatson
Copy link
Owner

JedWatson commented Nov 27, 2017

I'm planning to move this over to a css-in-js library for the next major version, but it will probably be something lighter-weight than styled-components (as much as I love that library, it adds a lot of overhead)

I'm also expecting to do some proper cleanup of how the internal components are organised and extendable so I wouldn't recommend anyone picking this up as a PR for now, there's a really high change it wouldn't end up being merged 😐

To hint at the future, it looks like we'll be using react-select in atlaskit which uses styled-components, so I understand the concerns @tobilen raises and whatever the next version is should be much more friendly to js-based themeing.

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

No branches or pull requests

4 participants