? ? Pending

User tests: Successful: Unsuccessful:

avatar dgrammatiko
dgrammatiko
31 Dec 2018

Pull Request for Issue # .

Summary of Changes

  • Eliminates jQuery
  • Removes the inline JS from the helper file, puts it in the layout

Testing Instructions

  • Make sure that custom fields are enabled, you have a custom field, etc
  • Create an article and switch categories (one that has custom fields to one that doesn't and the other way around)

Expected result

Actual result

Documentation Changes Required

No code clean up, enforce best practices, eg scripts only on the layouts!!!

avatar dgrammatiko dgrammatiko - open - 31 Dec 2018
avatar dgrammatiko dgrammatiko - change - 31 Dec 2018
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 31 Dec 2018
Category Administration com_categories com_fields
0f82861 31 Dec 2018 avatar dgrammatiko CS
avatar dgrammatiko dgrammatiko - change - 31 Dec 2018
Labels Added: ?
1b3ca96 31 Dec 2018 avatar dgrammatiko CS
avatar bembelimen
bembelimen - comment - 31 Dec 2018

Hi @dgrammatiko thank you for this PR, that's exactly where I tinkered around the last hours, but one suggestion: perhaps we could do this "reload" custom field independent in some way, (at least the naming of the parameter), because I would like to use this new function with the workflow.

avatar dgrammatiko
dgrammatiko - comment - 31 Dec 2018

@bembelimen you mean Joomla.categoryHasChanged(..) this one? This is an inline script available only in the edit views that have a category field.
Anyways what is you requirements for the workflow? Let me know what are your requirements so I could try to make this a more abstract method and maybe make it globally available (?)

avatar bembelimen
bembelimen - comment - 31 Dec 2018

The idea is nearly the same as custom fields.
At the moment when creating a new article there is no "state" field visible. I would like to show it again + the first transitions (so one can e.g. with the default workflow publish/unpublish a new article without saving twice). But every category could have their own workflow, so it has to change when changing the category.
I just need exactly the same what custom field does: relaod content and adjust the state field.
So my idea way a more generic approach (in fact what you implemented here), with a parameter, which allows me to activate this reload function when changing the category.
So codewise I think there is no changes required, it's more the naming (looks a bit strange, when the workflow activates a CF-function)

avatar dgrammatiko
dgrammatiko - comment - 31 Dec 2018

which allows me to activate this reload function when changing the category.

This is fired on category change so you don't have to call it from the workflow js... Or do I miss something?

avatar bembelimen
bembelimen - comment - 31 Dec 2018

Yes, that's clear.
Just to make it short: this function should be activatable from everywhere and from every extension, not only CF (which is now the case), so my suggestion was just: rename the parameters to be CF independent. (custom-fields-enabled)

avatar dgrammatiko
dgrammatiko - comment - 1 Jan 2019

@bembelimen so the method:

  Joomla.categoryHasChanged = function (el) {
	    if (el.value == el.getAttribute('data-cat-id')) {
	      return;
	    }
	
	    Joomla.loadingLayer('show');
	    document.body.appendChild(document.createElement('joomla-core-loader'));
	    document.querySelector('input[name=task]').value = el.getAttribute('data-section') + '.reload';
	    element.form.submit();
	  };

Basically has only this line: document.querySelector('input[name=task]').value = el.getAttribute('data-section') + '.reload'; that is specific to custom fields. I guess I can replace it with a callback to be more abstract. Only problem with the callback is that you can have only one. The other option is to dispatch an event so you can have multiple methods run at the event. Only draw back of the event is that is async so the form.submit() will have to go also form this method. If you can share some dummy code that workflows need to run here I could shape this...

avatar bembelimen
bembelimen - comment - 1 Jan 2019

Yes, this is the only line, which could be lead to a conflict.

To be honest, I have no code yet, I would just do, what you did here:

$form->setFieldAttribute('catid', 'custom-fields-enabled', true);
$form->setFieldAttribute('catid', 'custom-fields-cat-id', $assignedCatids);
$form->setFieldAttribute('catid', 'custom-fields-form-id', $form->getFormControl());
$form->setFieldAttribute('catid', 'custom-fields-section', $section);

And then manipulate over the loadForm-Hooks in the Model. As said before, I don't need "more" than this...and it would work like it is at the moment for me...but a more generic naming would be nice + the line mentioned by you could lead to problems (probably)

avatar joomla-cms-bot joomla-cms-bot - change - 1 Jan 2019
Category Administration com_categories com_fields Administration com_categories com_fields JavaScript Repository
avatar dgrammatiko
dgrammatiko - comment - 1 Jan 2019

@bembelimen ok I think you can easily plugin any needed functionality through either the data attributes (my approach here) or using the document->addScriptOptions().
Anyways for the time being the reload will also work for the forms without custom fields. The only part I guess you need to patch is to set in javascript whatever you need to re collect in the PHP on the form post.
Let me know if there is something more you need here

avatar bembelimen
bembelimen - comment - 1 Jan 2019

I would just suggest to rename the parameters to be CF-independent:

$data['enabledRelaod']   = (boolean) $this->element['reload-enabled'];
$data['catId']       = (string) $this->element['reload-cat-id'];
$data['formId']      = (string) $this->element['reload-form-id'];
$data['section']     = (string) $this->element['reload-section'];
avatar dgrammatiko
dgrammatiko - comment - 1 Jan 2019

@bembelimen better?

avatar bembelimen
bembelimen - comment - 1 Jan 2019

Worse :D the idea was to NOT use custom fields naming, but anyways, let it like it is, I'll use your naming.

avatar bembelimen
bembelimen - comment - 1 Jan 2019

I have tested this item successfully on 3d2680e


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

avatar bembelimen bembelimen - test_item - 1 Jan 2019 - Tested successfully
avatar chmst
chmst - comment - 1 Jan 2019

I have tested this item successfully on 3d2680e


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

avatar chmst chmst - test_item - 1 Jan 2019 - Tested successfully
avatar Quy Quy - change - 1 Jan 2019
Status Pending Ready to Commit
avatar Quy
Quy - comment - 1 Jan 2019

RTC


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

avatar dgrammatiko
dgrammatiko - comment - 4 Jan 2019
avatar wilsonge wilsonge - change - 4 Jan 2019
Labels Added: ?
avatar wilsonge wilsonge - change - 5 Jan 2019
Status Ready to Commit Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2019-01-05 18:26:31
Closed_By wilsonge
avatar wilsonge wilsonge - close - 5 Jan 2019
avatar wilsonge wilsonge - merge - 5 Jan 2019
avatar wilsonge
wilsonge - comment - 5 Jan 2019

Thanks!

Add a Comment

Login with GitHub to post a comment