search

nate-strauser / meteor-x-editable-bootstrap

Smart package for x-editable in place editor
80 stars 51 forks source link

Bootstrap 3 glyphicon icons #19

Open DSpeichert opened 10 years ago

nate-strauser commented 10 years ago

can you elaborate as to why this needs to be changed?

i'm hesitant to diverge from the x-editable code unless needed. it looks like x-editable still uses the older icon classes. https://github.com/vitalets/x-editable/blob/1.5.1/dist/inputs-ext/wysihtml5/bootstrap-wysihtml5-0.0.2/bootstrap-wysihtml5-0.0.2.js

DSpeichert commented 10 years ago

Yes, they do and it is bootstrap-3 incompatible. Those icons don't work in Bootstrap 3 because instead of "icon-" (Bootstrap 2), class names are now "glyphicon glyphicon-": http://getbootstrap.com/components/#glyphicons

nate-strauser commented 10 years ago

ok - understood - but shouldnt this be a pull request to x-editable then? - i dont want to diverge from the original/source plugin if at all possible

DSpeichert commented 10 years ago

They don't support Bootstrap 3 yet, while your package says:

NOTE: The latest branch uses the Bootstrap 3 build of x-editable. If you are still using Bootstrap 2, install v1.4.6.3 instead of the latest.

I was confused about where is that Bootstrap 3 support. I guess that until they are fully Bootstrap 3 - compatible, it's ok to diverge a little and then go back once they made necessary changes.

nate-strauser commented 10 years ago

weird thing is that it says BS3 all over the front page - http://vitalets.github.io/x-editable/

On Mon, Jun 16, 2014 at 12:09 PM, Daniel Speichert <notifications@github.com

wrote:

They don't support Bootstrap 3 yet, while your package says:

NOTE: The latest branch uses the Bootstrap 3 build of x-editable. If you are still using Bootstrap 2, install v1.4.6.3 instead of the latest.

I was confused about where is that Bootstrap 3 support. I guess that until they are fully Bootstrap 3 - compatible, it's ok to diverge a little and then go back once they made necessary changes.

— Reply to this email directly or view it on GitHub https://github.com/nate-strauser/meteor-x-editable-bootstrap/pull/19#issuecomment-46198876 .

DSpeichert commented 10 years ago

They also have Boostrap 3 support on the roadmap: https://github.com/vitalets/x-editable/wiki/Roadmap

Since the class names are different, it's not possible to support both versions of Bootstrap at the same time.

nate-strauser commented 10 years ago

from looking at their grunt file, it sure looks like they have bs3 specific files - seems like they would just need a bs3 version of the file in question, including that version when building for bs3

On Mon, Jun 16, 2014 at 12:52 PM, Daniel Speichert <notifications@github.com

wrote:

They also have Boostrap 3 support on the roadmap: https://github.com/vitalets/x-editable/wiki/Roadmap

Since the class names are different, it's not possible to support both versions of Bootstrap at the same time.

— Reply to this email directly or view it on GitHub https://github.com/nate-strauser/meteor-x-editable-bootstrap/pull/19#issuecomment-46203941 .

DSpeichert commented 10 years ago

It looks like the issue is already there: https://github.com/vitalets/x-editable/issues/606