search

lightyrs / tabler-rubygem

Rubygem for https://tabler.github.io
MIT License
82 stars 27 forks source link

Remove explicit sass usage and add bootstrap as a dependency #15

Closed vbalazs closed 5 years ago

vbalazs commented 5 years ago

Builds on #14 (hence the commits in the diff) Fixes #13

I'm still unsure about adding bootstrap as a dependency because this way we might lock the version and users cannot upgrade it independently if 4.4.x comes out. :| But this seems more correct to me. As a middle ground, we could be more liberal and set the version as ~> 4.3? 🤔 What do you think?

lightyrs commented 5 years ago

@vbalazs Have you tried running the update rake task with your new code. Does it work correctly?

vbalazs commented 5 years ago

@lightyrs yep, as far as I can judge the output. It didn't fail and updated many files, also the value of TABLER_SHA in lib/tabler/rubygem/version.rb

rbngzlv commented 5 years ago

Hi @vbalazs and @lightyrs,

I'm not sure to adding bootstrap as a runtime dependency in the gem, because the user can provide it in another way, for example, we are loading bootstrap from node_modules and not the bootstrap gem, so we will end with two bootstrap versions installed. 

// app/assets/stylesheets/application.scss
@import "tabler/variables"; // this gem
@import "bootstrap/scss/bootstrap"; // from node_modules
@import "tabler"; // this gem

I forked the @vbalazs fork and made a branch based on their other PR #14, to fix the sass usage, this is de diff: https://github.com/vbalazs/tabler-rubygem/compare/fix-dev-env...rbngzlv:rbngzlv/remove-sass

What do you think?

P.D: thank you both for the work on this

lightyrs commented 5 years ago

This seems like a legitimate issue @rbngzlv — Please submit your branch as a PR.