? Pending

User tests: Successful: Unsuccessful:

avatar dgt41
dgt41
29 Nov 2017

Pull Request for Issue # .

Summary of Changes

Changes for the com_wrapper and mod_wrapper to use custom elements.

This fixes:

  • Inability to use multiple modules in the same page (id was hardcoded)
  • New code follows the progressive enhancement (part of the PWA) technic, eg iframe will be viewable (but not with the desired height) if javascript is disabled.
  • The javascript is not blocking the rendering of the page as (and this goes for all custom elements) it's loaded asynchronously after the initial page render!

Testing Instructions (by @C-Lodder)

  • Create 2 wrapper modules
  • assign to the same page
  • ensure they work as expected

Expected result

Actual result

screen shot 2017-11-29 at 16 23 27

Documentation Changes Required

None. The new code reflects the old behaviour!

avatar dgt41 dgt41 - open - 29 Nov 2017
avatar dgt41 dgt41 - change - 29 Nov 2017
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 29 Nov 2017
Category JavaScript Repository Front End com_wrapper Modules
avatar C-Lodder
C-Lodder - comment - 29 Nov 2017

Sometimes wish iframes would just become obselete

avatar dgt41 dgt41 - change - 29 Nov 2017
Labels Added: ?
avatar dgt41 dgt41 - change - 29 Nov 2017
The description was changed
avatar dgt41 dgt41 - edited - 29 Nov 2017
avatar brianteeman
brianteeman - comment - 29 Nov 2017

and when you look at where we use them in the admin ui you will see how abused they are and how they are not used correctly.

avatar dgt41
dgt41 - comment - 29 Nov 2017

@brianteeman shall we do the a11y part in another PR?

avatar brianteeman
brianteeman - comment - 29 Nov 2017

i dont see why. this is a total replacement of the current iframe code so why leave it til after this one has been merged. its just duplicating work for you and for testers.

avatar dgt41
dgt41 - comment - 29 Nov 2017

Ok, so title will be the menu title for the component and the module title for the modules?

avatar brianteeman
brianteeman - comment - 29 Nov 2017

For _wrapper then yes that will should be ok but note it won't be in other places in the core that we have iframe it won't be

avatar franz-wohlkoenig
franz-wohlkoenig - comment - 9 Dec 2017

@dgt41 can this PR be tested, can you give Test Instructions?


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

avatar franz-wohlkoenig franz-wohlkoenig - change - 9 Dec 2017
Status Pending Information Required
avatar franz-wohlkoenig
franz-wohlkoenig - comment - 24 Dec 2017

Reminder for @dgt41


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

avatar C-Lodder
C-Lodder - comment - 24 Dec 2017
  • Create 2 wrapper modules
  • assign to the same page
  • ensure they work as expected
avatar franz-wohlkoenig
franz-wohlkoenig - comment - 24 Dec 2017

thanks for Info, @C-Lodder (i added them in first Comment so Tester don't have to search in Thread).

@dgt41 can you please resolve conflicting Files so this Pull Request can be tested?

avatar joomla-cms-bot joomla-cms-bot - edited - 24 Dec 2017
avatar franz-wohlkoenig franz-wohlkoenig - change - 24 Dec 2017
The description was changed
avatar dgt41
dgt41 - comment - 24 Dec 2017
avatar franz-wohlkoenig franz-wohlkoenig - change - 24 Dec 2017
Status Information Required Pending
avatar franz-wohlkoenig franz-wohlkoenig - test_item - 24 Dec 2017 - Tested successfully
avatar franz-wohlkoenig
franz-wohlkoenig - comment - 24 Dec 2017

I have tested this item successfully on fcb898e

bildschirmfoto 2017-12-24 um 12 14 56


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/18916.
avatar Anu1601CS
Anu1601CS - comment - 5 Mar 2018

@dgt41 can you resolve these conflicting files.
So we can test this. There was some work pending on Joomla-es6 repo
Thanks

avatar dgt41
dgt41 - comment - 6 Mar 2018

@Anu1601CS done

avatar Anu1601CS
Anu1601CS - comment - 6 Mar 2018

I can't test. :(

screenshot from 2018-02-07 22-37-39

let this PR to merge #19836

@franz-wohlkoenig , @astridx can you test again if you are not getting this issue .

avatar laoneo
laoneo - comment - 7 Mar 2018

@Anu1601CS does that happen when you save a module?

avatar Anu1601CS
Anu1601CS - comment - 7 Mar 2018

@laoneo yes

avatar laoneo
laoneo - comment - 7 Mar 2018

Can you test if my pr is working?

avatar astridx
astridx - comment - 8 Mar 2018

I have got problems with this brach.
After applying I see an error message in both wrapper modules and I can see only the second iframe.
home 1


Error Message
Notice: Undefined variable: title in /var/www/html/JOOMLA/joomla4/joomla-cms/modules/mod_wrapper/tmpl/default.php on line 24 Call Stack #TimeMemoryFunctionLocation 10.0001363648{main}( ).../index.php:0 20.0001364128require_once( '/var/www/html/JOOMLA/joomla4/joomla-cms/includes/app.php' ).../index.php:36 30.01511397824Joomla\CMS\Application\SiteApplication->execute( ).../app.php:38 40.09532786032Joomla\CMS\Application\SiteApplication->render( ).../CMSApplication.php:351 50.09542786056Joomla\CMS\Application\SiteApplication->render( ).../SiteApplication.php:754 60.10042820232Joomla\CMS\Document\HtmlDocument->render( ).../CMSApplication.php:1083 70.10042820232Joomla\CMS\Document\HtmlDocument->_renderTemplate( ).../HtmlDocument.php:555 80.10612886528Joomla\CMS\Document\HtmlDocument->getBuffer( ).../HtmlDocument.php:780 90.10612886608Joomla\CMS\Document\Renderer\Html\ModulesRenderer->render( ).../HtmlDocument.php:489 100.10792933800Joomla\CMS\Document\Renderer\Html\ModuleRenderer->render( ).../ModulesRenderer.php:47 110.10802936432Joomla\CMS\Helper\ModuleHelper::renderModule( ).../ModuleRenderer.php:98 120.10812957080include( '/var/www/html/JOOMLA/joomla4/joomla-cms/modules/mod_wrapper/mod_wrapper.php' ).../ModuleHelper.php:204 130.10822958312require( '/var/www/html/JOOMLA/joomla4/joomla-cms/modules/mod_wrapper/tmpl/default.php' ).../mod_wrapper.php:26 " iframe-class="wrapper">

avatar BaleshSrle
BaleshSrle - comment - 2 May 2018

@C-Lodder Why do people need to complicate things, and don't want to make it simpler?
@dgrammatiko There will not be any merging, because this uses old HTML4 standard, and it will be extremley dificult for me to modify this heap of files to new HTML5 standard.
BTW, I created a similar PR, but for Joomla 3.8 and I can inform you that my solution works on my 2 web-sites. #19965

avatar dgrammatiko
dgrammatiko - comment - 2 May 2018

@BaleshSrle can you explain me what is complicate here? I'm just using the W3C standards Custom Elements, is that complicated? If so you should rethink your involvement
with the web.

Anyways I've asked people with merge right to first merge your PR and then I'll patch things here

avatar C-Lodder
C-Lodder - comment - 2 May 2018

@BaleshSrle - I have no idea what you're on about and why you're asking me

avatar BaleshSrle
BaleshSrle - comment - 3 May 2018

@C-Lodder I don't know, maybe because I can see widsom on you.
@dgrammatiko I looked at the heap of files that you created and I can honestly say that Joomla 4 will be one heavy CMS to upload (unzipped maybe over 25 to 30 MB). And yes, you are using W3C's old HTML4 standard, but I'm using W3C's new HTML5 standard.

avatar laoneo
laoneo - comment - 9 Aug 2018

Can you fix the conflicts on this one please?

3ba207f 9 Aug 2018 avatar dgrammatiko init
avatar dgrammatiko
dgrammatiko - comment - 9 Aug 2018

@laoneo done

avatar dgrammatiko
dgrammatiko - comment - 10 Aug 2018

Preview component + module (inception mode):
screenshot 2018-08-10 at 21 52 03

avatar brianteeman
brianteeman - comment - 10 Aug 2018

Can you remove the commented out code please

avatar dgrammatiko dgrammatiko - change - 29 Aug 2018
Status Pending Closed
Closed_Date 0000-00-00 00:00:00 2018-08-29 10:23:01
Closed_By dgrammatiko
avatar dgrammatiko dgrammatiko - close - 29 Aug 2018

Add a Comment

Login with GitHub to post a comment