search

cibernox / ember-power-select

The extensible select component built for ember.
http://www.ember-power-select.com
Other
540 stars 377 forks source link

improve bootstrap theme #44

Closed miguelcobain closed 6 years ago

miguelcobain commented 8 years ago

I think we're not quite there yet:

seven7seven commented 8 years ago

Also, the Bootstrap ecosystem is very LESS-centric, using Sass doesn't integrate very well. If possible, I would also provide a LESS version of the main stylesheet.

cibernox commented 8 years ago

@seven7seven Bootstrap 4 has abandoned less in favour of sass, and bootstrap 3 had an official sass fork for as long as I can remember.

I think that with sass and pure css we cover enough options. Is it even possible to transform sass to less?

miguelcobain commented 8 years ago

Bootstrap 4 is moving from less to sass. Still, I think a LESS version would be cool to let people choose their favorite styles preprocessor. Keep in mind that we can't possibly support all the styles preprocessors in the world.

seven7seven commented 8 years ago

Ah, I guess if they're also switching to Sass, it'll work out. I think Sass converts to LESS trivially.

Yeah, LESS if probably still going to be around for a while. I'm trying to see if I can integrate ember-power-select with my workflow, otherwise I'll consider a PR for .less support.

seven7seven commented 8 years ago

Ah, I see now, the blueprint added an app.scss file; that's a little weird -- since I was using app.less exclusively.

Could the blueprint possibly check and automatically toggle between .less and .scss?

cibernox commented 8 years ago

@seven7seven That looks like a bug to me. We should not add a app.scss file if you're not using sass (that is easy to detect). Actually the ideal solution would be to just append a couple lines to your already existing app.scss instead of trying to replace the file.

I'll add it as an issue. And I'll add as an issue the less support flagged with the help wanted flag so anyone can take it.

miguelcobain commented 8 years ago

Actually the ideal solution would be to just append a couple lines to your already existing app.scss instead of trying to replace the file.

That part is (kind of) already handled. Ember-cli will ask you if you want to replace your file, allowing you to see a diff of both. It doesn't replace a whole file silently. It complains loudly, so I guess that's not entirely bad.

seven7seven commented 8 years ago

@cibernox sure, Ref the issue here as well please, I'm already on it.

My LESS pipeline really can't deal with a separate Scss file -- I'm importing bootstrap, heavily customizing its variables, and then I need those variables to be applied to the select's theme.

Attempting to mimic the style of the BS3 theme, without taking bootstrap's variables (e.g. border radius, main color, shadow, placeholder color etc.) into account is kind of odd to begin with.

miguelcobain commented 8 years ago

Attempting to mimic the style of the BS3 theme, without taking bootstrap's variables (e.g. border radius, main color, shadow, placeholder color etc.) into account is kind of odd to begin with.

I feel you. We should use either bootstrap classes or bootstraps own variables, is that right?

cibernox commented 8 years ago

I see. That is a bit more involved of what I thought but is doable as long as the bootstrap variables are imported before the bootstrap theme.

I feel more confortable customizing sass/less with variables than with using bootstrap class names in the template, but I'm not sure if you can achieve full customization without them.

seven7seven commented 8 years ago

@miguelcobain Yep. Classes for stuff that already exists / is trivial, like help-text, forms etc., and variables to normalize style and add flair, like the line-height of dropdown items for example.

You can't really do this with the current setup, unless you use bootstrap's Sass + some trickery.

seven7seven commented 8 years ago

@cibernox Actually you're right, it's better to just use variables in the custom theme file, which is optional.

You can also add general classes, like .ember-select-input, and then in the custom bootstrap theme file, do something like:

.ember-select-input {
  .form-input;
}
cibernox commented 8 years ago

@seven7seven In case you want to help I've created #57 to add basic less support (compile from sass to less).

A fully refined bootstrap theme might take a bit longer to get it right and I won't release 1.0 without it.

seven7seven commented 8 years ago

Sounds good.

Right now I'm rushing to migrate (the hell away) from ember-select-2 and get my project on the 1.13 train. I might cut some corners (e.g. include the ember-basic dropdown .scss as converted .less, inside the ember-power-select.less), but if this select is right for us, I'll put in work to get a solid bootstrap theme.

cibernox commented 8 years ago

In case it's not right for your use case for some reason please let me know any problem you've found

jcbvm commented 8 years ago

One other thing is the disabled state, it should show a different cursor. And another nice addition would be to set the size of the trigger, like sm and lg, corresponding to input-sm and input-lg of bootstrap.

fran-worley commented 7 years ago

I'm bumping this following the release of the public beta of Bootstrap 4.

It strikes me that a key issue holding back improvements here in the intent to do everything via setting variables, as opposed to simply having specific stylesheets for frameworks like bootstrap. The benefit of the latter is that you can re-use variables from the framework rather than users having to set a myriad of different variables specific to your library.

I began migrating to this project from selectize for the improved 'emberness' and am now having to recreate bootstrap styling to match that used by selectize (which is far superior in my view). I'm happy to PR my bootstrap4 code for anyone who is interested.

cibernox commented 7 years ago

@fran-worley Perhaps you want to take at look at https://github.com/kaliber5/ember-bootstrap It has a select based on ember-power-select but with a better customization than the one you can achieve just by dropping styles.

fran-worley commented 7 years ago

Thanks @cibernox but that project tries to emberify the entirety of the bootstrap framework and I get pretty nervous of projects like that having been badly stung on an old rails project.

They appear to be having a bit of a nightmare with bootstrap 4 so I think I'm probably better off fixing the styling of this addon. I was more wondering whether you'd be interested in merging something for BS4 given that you have something in place for BS3.

cibernox commented 7 years ago

Sure, I'd be glad to see what do you have for bootstrap 4. The bootstrap 3 and 4 themes can coexist if we name the new one "bootstrap-4"

2017-08-14 18:07 GMT+02:00 Fran Worley notifications@github.com:

Thanks @cibernox https://github.com/cibernox but that project tries to emberify the entirety of the bootstrap framework and I get pretty nervous of projects like that having been badly stung on an old rails project.

They appear to be having a bit of a nightmare with bootstrap 4 so I think I'm probably better off fixing the styling of this addon. I was more wondering whether you'd be interested in merging something for BS4 given that you have something in place for BS3.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/cibernox/ember-power-select/issues/44#issuecomment-322233237, or mute the thread https://github.com/notifications/unsubscribe-auth/AAQMe0dUPvNH8O0mGmzIJddOyjHlEQd9ks5sYHDMgaJpZM4GPXgt .

fran-worley commented 7 years ago

@cibernox I've left this as a gist rather than a PR as it's probably not in a format you'd want to formally release, particularly if you're set on everything having a variable.

https://gist.github.com/fran-worley/cbca91417863b44bb34b8f1e171c01cc