Closed trentclowater closed 8 years ago
Hi trentclowater!
You're totally right, I have to fix this behavior ASAP. I'll change the bootstrap dependency as you suggest in the second option and I'll publish a new version of the gem with this and merging your new PR.
Thank you for your effort and suggestions :)
Thanks for making the gem in the first place!
Note that there is also a jquery/jquery.min.js
file that doesn't seem to be referenced anywhere, and seems like it could be removed.
Also the RTL version might be a bit of a problem, since I didn't realize Bootstrap 3 doesn't actually have RTL support. After checking around a bit, I think I would recommend dropping the Bootstrap import for the RTL version also, but including the styles from https://github.com/morteza/bootstrap-rtl
, which is an extension that sits on top of the original Bootstrap files. This way bootstrap could still be updated using the official gem separately for the RTL version.
And finally, even though I just made all the changes to include the latest version of Font Awesome, it has a well-maintained and up-to-date gem also, so it might be good to remove it from here and just include it as a dependency as well.
If you like I can do a PR for this myself, or I can help you out if you do it. Just let me know.
Don't worry, this week I have some free time so I can fix and change all this details, but if I need help I'll let you know.
Thank you for suggest all of this, they are details that I didn't had in mind in the moment of develop the gem, but it needs to be changed. I really appreciate it :)
Hi,
I see you did some refactoring on the gem, which will make it much easier to keep up with changes to FontAwesome! The original issue I mentioned when opening this issue still exists, though, which means the instructions provided in the README
don't actually work (the main Bootstrap JS file isn't included). Do you have plans to do another update for this?
Even just adding the bootstrap.min.js
file to the bootstrap_sb_admin_base_v2
manifest file would make things work according to the README
, and make things consistent.
Also, you seem to have accidentally deleted the glyphicon font files when you deleted the FontAwesome files. The glyphicon files are required for Bootstrap.
Hi trentclowater!
Sorry for the late reply, I've been very busy this last two weeks. I just restore the glyphicon font files, because you're right, they had been deleted by mistake. I also made the changes you mention and I added the javascript file for bootstrap (so it should work according to the README
instructions), and I upgraded Bootstrap version to 3.3.7
If you see more inconsistences or bugs, just let me know in order to fix it.
Thank you very much for all this great feedback! 😊
Thanks for taking care of this!
Hi,
Currently this gem includes the main bootstrap .scss files, but not the bootstrap .js files. This results in a few different scenarios:
I think it would be better to either include both the .js and .scss files, or not include the main bootstrap files at all and include bootstrap-sass as a dependency (with installation instructions on how to include them in the manifest files in the readme.) Personally I would prefer the second option. What do you think?