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

Bootstrap 3 glyphicon icons #19

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

DSpeichert
Copy link

No description provided.

@DSpeichert DSpeichert changed the title Bootstrap 3 glypicon icons Bootstrap 3 glyphicon icons Jun 15, 2014
@nate-strauser
Copy link
Owner

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
Copy link
Author

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
Copy link
Owner

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
Copy link
Author

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
Copy link
Owner

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 <[email protected]

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
#19 (comment)
.

@DSpeichert
Copy link
Author

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
Copy link
Owner

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 <[email protected]

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
#19 (comment)
.

@DSpeichert
Copy link
Author

It looks like the issue is already there: vitalets/x-editable#606

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

Successfully merging this pull request may close these issues.

2 participants