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

Move plugins modals to Bootstrap #4664

Closed
wants to merge 2 commits into from
Closed

Move plugins modals to Bootstrap #4664

wants to merge 2 commits into from

Conversation

dgrammatiko
Copy link
Contributor

Moving current code to bootstrap modal

Same as #4661 #4645 #4575 #4563 #4561 #4514 #4513

Testing:
After Applying this patch
Try editing an article and use the editor’s buttons

Preview:
screenshot 2014-10-14 07 48 07
screenshot 2014-10-14 07 48 24
screenshot 2014-10-14 07 48 39

==== Moving current code to bootstrap modal

Same as #4661 #4645 #4575 #4563 #4561 #4514 #4513
@dgrammatiko
Copy link
Contributor Author

This one AS IT IS BREAKS B/C.
All needed is a proxy from
SqueezeBox.close();
to
jQuery("#menusModuleModal").modal("hide”);
Anyone?

@Fedik
Copy link
Member

Fedik commented Oct 14, 2014

I do not see b/c problem in replace SqueezeBox.close(); to jQuery("#menusModuleModal").modal("hide”); ...
or you got some problem with it?

@infograf768
Copy link
Member

@test

results here (Firefox Macintosh) are quite frustrating:
browser window set to 744px:
744px
Browser window set to 830px
830px

@infograf768
Copy link
Member

Forget it. This one is missing the css found in #4514
once the css are in. it's OK

@dgrammatiko
Copy link
Contributor Author

@Fedik The problem will be as follow:
Someone got a button from a 3rd party dev.
This button does some fancy staff and on save it closes the modal.
Ooops problem because calling SqueezeBox.close(); won’t do anything.
I STRONGLY believe that we should (only on this PR) have a proxy for the old close function
Unless if we are SURE that NOBODY ever calls this function [honestly I don’t use those buttons at all so I am not the one to certify that]

@dgrammatiko
Copy link
Contributor Author

Update @Fedik Looking at the available buttons on extensions directory there is no such plugin available, so I have to admit that I WAS WRONG.

@dgrammatiko
Copy link
Contributor Author

@infograf768 I didn’t push the less/css changes in every PR for the shake of simplicity. I will update the description for a proper way of testing

@Fedik
Copy link
Member

Fedik commented Oct 14, 2014

@dgt41 even if someone use it, he/she use it with behavior.modal and not with bootstrap.modal, as I understand 😄

@trangredweb
Copy link

@test

@lunguyenhat
Copy link

@test

This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/4664.

@kienlc
Copy link

kienlc commented Oct 17, 2014

@test

@rajeshstarlite
Copy link

@test, Modal box works fine in my test but in IE8 some CSS problem. Please take a look at attached screen shot.
screen shot 2014-10-17 at 02 26 10

This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/4664.

@gunjanpatel
Copy link
Contributor

@luredweb and @trangredweb can you please confirm that you are not able to reproduce the issue on IE8. Because, @ rajeshstarlite experienced the issue with IE8. Please confirm, so we may ready to commit. Right now it's not possible to move RTC.
Thanks all.

This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/4664.

@micker
Copy link

micker commented Oct 17, 2014

i can't test it

_Error

The patch could not be applied because it conflicts with a previously applied patch: administrator/templates/isis/css/template-rtl.css_


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/4664.

@gunjanpatel
Copy link
Contributor

@micker try to apply patch manually using https://github.com/joomla/joomla-cms/pull/4664.diff or you may try to revert your other patch which you have applied.

This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/4664.

@lunguyenhat
Copy link

yes, @gunjanpatel, I tested IE8 it's some CSS problem in IE8 as @rajeshstarlite

This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/4664.

@brianteeman
Copy link
Contributor

@micker you need to always revert a patch before applying another one - this makes sure that you are always testing just one patch

This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/4664.

@dgrammatiko
Copy link
Contributor Author

@ALL IE8 doesn’t support css3, thus the sharp angles. This is also the way Bootstrap itself works. I don’t think we should invest time trying to make bootstrap modals look good on IE8

@gunjanpatel
Copy link
Contributor

Thanks. Moving to RTC as we have more than 3 successfull tests.

@gunjanpatel
Copy link
Contributor

Sorry for typo in above comment. We have 3 successfull test so moving RTC. :)

This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/4664.

@dgrammatiko
Copy link
Contributor Author

@brianteeman and the rest of the PLT:
WARNING
This PR might have negative impact in the front end templates, if no bootstrap css is loaded!

@dgrammatiko
Copy link
Contributor Author

I am closing this. PLEASE DO NOT COMMIT.

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.

None yet