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

RFC Batch modals comply with the new layout #7000

Closed
wants to merge 5 commits into from
Closed

RFC Batch modals comply with the new layout #7000

wants to merge 5 commits into from

Conversation

dgrammatiko
Copy link
Contributor

Batch modals are hardcoded. This is a proposal to make them more flexible/overridable. Right now it’s only for banners, but, if this gets some attention I will make it site wide.

So what’s in it?

Well batch modals have a tmpl/default_batch.php file that holds all the html code for the modal.
But since @smz and @phproberto made some nice changes in the bootstrap modal everything is more modular and everything is controlled from the respected layouts e.g. body, footer.
So this PR tries to break the hardcoded html code to 2 files tmpl/default_batch_body.php and tmpl/default_batch_footer.php
Now we have more control over the output (read this as getting ready for bs3)

@dgrammatiko dgrammatiko changed the title RC RFC Batch modals May 20, 2015
@dgrammatiko dgrammatiko changed the title RFC Batch modals RFC Batch modals comply with the new layout May 21, 2015
@ghost
Copy link

ghost commented May 21, 2015

@test

Template Isis Success with current Firefox, IE11, current Chrome. and com_banners.
Batched several items succesfully. Could cancel, close etc.

Template Hathor: Error 500 Layout default_batch not found.

batchbanner-hathor

@Kubik-Rubik
Copy link
Member

@test Tested successfully with both administrator templates. Batch feature still works properly!


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

@MATsxm
Copy link

MATsxm commented May 21, 2015

@test

#7000 Tested successfully with both administrator templates.


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

@smz
Copy link
Contributor

smz commented May 21, 2015

@test OK (both admin templates)

@dgt41
I think it is not necessary to set the width and height parameters options for the modal: if I remember correctly those are used only in case of an iframe modal, which is not the case here...

@Kubik-Rubik
Copy link
Member

@smz Sergio, if you test it, then please mark your result in the issue tracker as well (not only in a comment at GitHub).

@dgt41 Thank you for your contribution. I will merge it!


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

@smz
Copy link
Contributor

smz commented May 21, 2015

@Kubik-Rubik ... oops, forgot about it, sorry!

@Kubik-Rubik
Copy link
Member

@smz Sergio, no problem at all, just a small reminder! :-)

@Kubik-Rubik Kubik-Rubik added this to the Joomla! 3.4.2 milestone May 21, 2015
@Kubik-Rubik Kubik-Rubik added the RTC This Pull Request is Ready To Commit label May 21, 2015
@smz
Copy link
Contributor

smz commented May 21, 2015

@Kubik-Rubik Victor, honestly I'd removed those width and height options before merging...

@smz
Copy link
Contributor

smz commented May 21, 2015

... but they do no harm: just ignored.

@Kubik-Rubik
Copy link
Member

@smz Since I've merged it already, please do a small PR for it. In #7003 @dgt41 should remove them before merging. Thanks!

@smz
Copy link
Contributor

smz commented May 21, 2015

OK!

@smz
Copy link
Contributor

smz commented May 21, 2015

... or @dgt41 could incorporate that in #7003. I stand-by waiting for @dgt41...

@dgrammatiko
Copy link
Contributor Author

@smz @Kubik-Rubik will do it in #7003, thanks!

@dgrammatiko dgrammatiko deleted the _rfc_batch_modals branch May 22, 2015 00:29
johanjanssens pushed a commit to joomlatools/joomla-fork that referenced this pull request Jun 19, 2015
@zero-24 zero-24 removed the RTC This Pull Request is Ready To Commit label Oct 14, 2015
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

6 participants