? Pending

User tests: Successful: Unsuccessful:

avatar dgrammatiko
dgrammatiko
4 Oct 2018

Pull Request for Issue # .

Summary of Changes

  • Drops the code from the core.js for the spinner (already deprecated in j3) and introduces a web component
  • Some minor changes in the build module for the web components. The settings.json will now allow custom paths in the media folder. So someone could do some PRs to move the fields custom elements to their appropriate place (/media/system/js/fields/)
  • Seperated the logic from the mark up for the category field (someone really needs to do this for all the mark up that is hidden in fields or some helpers!!!)
  • Reworked the custom fields part that injected some inline jQuery script. So now the script is part of the category field (that is the right place for the js to live) and the helper just passes some data to the field. The js is still inline so someone should just move it to a file...
  • Patched the custom fields to work with the new element
  • Patched the installation to work with the new element
  • Patched the com_associations to work with the new element

Testing Instructions

Install Joomla, spinner is ok
Make sure that you have custom fields assigned to one category. Edit an article and change its cat to the one that custom fields are attached. spinner is ok
Try a multilingual and check if the spinner works ok

Expected result

Actual result

Documentation Changes Required

In 4.0 in each page that a spinner is needed the following steps will need to be done:

  • Include the web component
HTMLHelper::_('webcomponent', 'system/webcomponents/joomla-core-loader.min.js', ['relative' => true, 'version' => 'auto']);
  • To show the spinner all we need to do is create an element:
spinner = document.createElement('joomla-core-loader');

// Assuming that the spinner is full screen
document.body.appendChild(spinner);

/*
if we need to append the spinner to some other container
all we have to do is replace document.body with the element 
that we want the spinner to appear
*/
  • To remove the spinner all we need to do is destroy the element:
spinner = document.querySelector('joomla-core-loader');

spinner.parentNode.removeChild(spinner);

if you want to use the same codebase for different versions (j3 and j4) here is a quick code:

if (Joomla.loadingLayer && typeof Joomla.loadingLayer === 'function') {
 // We are in J3 so use the old method
 Joomla.loadingLayer('show');
} else {
 // We are in the future
spinner = document.createElement('joomla-core-loader');
document.body.appendChild(spinner);
}
avatar dgrammatiko dgrammatiko - open - 4 Oct 2018
avatar dgrammatiko dgrammatiko - change - 4 Oct 2018
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 4 Oct 2018
Category Administration com_associations com_categories com_fields Modules JavaScript Repository Installation Layout
avatar dgrammatiko dgrammatiko - change - 4 Oct 2018
Labels Added: ?
avatar brianteeman
brianteeman - comment - 4 Oct 2018

There are multiple issues being addressed in this one pr and it really should be one issue per pr. It makes testing much more reliable and avoids parts of the pr not being tested etc

avatar dgrammatiko
dgrammatiko - comment - 4 Oct 2018

It’s a take it or leave it...

avatar brianteeman
brianteeman - comment - 4 Oct 2018

Thats a shame as its not the right thing to do (as you well know) and its the reason why whe in the past pr's with multiple issues were merged we are wasting time undoing all the mistakes.

avatar Fedik
Fedik - comment - 4 Oct 2018

maybe can you please skip the category field for now? it will have a huge conflict with #22263

avatar dgrammatiko
dgrammatiko - comment - 4 Oct 2018

@Fedik unfortuanatelly this is a really poor implementation with so many anti patterns so at some point it needs to be done.

  • field markup is not overridable
  • field specific js is loaded in a helper file in com_fields

I know it’s a pain but at some point these needs to be done correctly...

avatar brianteeman
brianteeman - comment - 4 Oct 2018

As I am currently wasting a lot of time redoing a lot of work that was committed in error because it was not tested properly because it was buried in a big pr with other things we must stop accepting any pr that addresses multiple issues at the same time. It is a waste of people's limited time to have to redo things that were not able to be tested!

0edb8d4 4 Oct 2018 avatar dgrammatiko cs
avatar joomla-cms-bot joomla-cms-bot - change - 4 Oct 2018
Category Administration com_associations com_categories com_fields Modules JavaScript Repository Installation Layout Administration com_associations com_categories com_fields JavaScript Repository Installation Layout
avatar dgrammatiko dgrammatiko - change - 4 Oct 2018
Status Pending Closed
Closed_Date 0000-00-00 00:00:00 2018-10-04 09:54:29
Closed_By dgrammatiko
avatar dgrammatiko dgrammatiko - close - 4 Oct 2018
avatar dgrammatiko
dgrammatiko - comment - 4 Oct 2018

Ok good luck

Add a Comment

Login with GitHub to post a comment