? ? Pending

User tests: Successful: Unsuccessful:

avatar Fedik
Fedik
16 Jan 2021

Pull Request for Issue #18843 and only for J3 .

Summary of Changes

This is fork of #18843, with requested changes.
The code make sure that showon initialised only once, no mater how often Joomla.setUpShowon was called.
This is achieved with extra flag showonInitialised that sets while showon initialisation.

Testing Instructions

Make sure showon still works.

Example, go to global config change "Debug Language" on/off, should downup/hides "Language Display" field.
Also the field "Language Display" will have extra attribute data-showon-initialised

Actual result BEFORE applying this Pull Request

showon works,
the showon field do not have data-showon-initialised

Expected result AFTER applying this Pull Request

showon works,
the showon field do have data-showon-initialised attribute

avatar Fedik Fedik - open - 16 Jan 2021
avatar Fedik Fedik - change - 16 Jan 2021
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 16 Jan 2021
Category JavaScript
avatar richard67
richard67 - comment - 16 Jan 2021

@Fedik #18843 can be closed, right?

avatar Fedik
Fedik - comment - 16 Jan 2021

yeah, I have included changes from that pull

avatar dgrammatiko
dgrammatiko - comment - 16 Jan 2021

@Fedik diffing the changes with the v4 version of show on I think you need to rename this to Joomla.Showon.initilise to be called like Joomla.Showon.initilise(field). The initialise in J4 could be a function in the Class with this code (the refactoring should be straight forward):

const jsondata = field.getAttribute('data-showon') || '';
const showonData = JSON.parse(jsondata);
let localFields;
if (showonData.length) {
localFields = [].slice.call(self.container.querySelectorAll(`[name="${showonData[0].field}"], [name="${showonData[0].field}[]"]`));
if (!this.fields[showonData[0].field]) {
this.fields[showonData[0].field] = {
origin: [],
targets: [],
};
}
// Add trigger elements
localFields.forEach((cField) => {
if (this.fields[showonData[0].field].origin.indexOf(cField) === -1) {
this.fields[showonData[0].field].origin.push(cField);
}
});
// Add target elements
this.fields[showonData[0].field].targets.push(field);
// Data showon can have multiple values
if (showonData.length > 1) {
showonData.forEach((value, index) => {
if (index === 0) {
return;
}
localFields = [].slice.call(self.container.querySelectorAll(`[name="${value.field}"], [name="${value.field}[]"]`));
if (!this.fields[showonData[0].field]) {
this.fields[showonData[0].field] = {
origin: [],
targets: [],
};
}
// Add trigger elements
localFields.forEach((cField) => {
if (this.fields[showonData[0].field].origin.indexOf(cField) === -1) {
this.fields[showonData[0].field].origin.push(cField);
}
});
// Add target elements
if (this.fields[showonData[0].field].targets.indexOf(field) === -1) {
this.fields[showonData[0].field].targets.push(field);
}
});
}

avatar Fedik
Fedik - comment - 16 Jan 2021

This only for J3, it should not go to J4,
there already event stuff for such thing:

/**
* Initialize 'showon' feature when part of the page was updated
*/
document.addEventListener('joomla:updated', ({ target }) => {

Only missed check for "double initialisation", I can do it later in another pull.

I do not think that this patch is much needed, but people asked ?
It can be safely closed, depend what @HLeithner will decide

avatar HLeithner
HLeithner - comment - 16 Jan 2021

@Fedik would be cool if we have the double initialization added in this pr too. After 2 tests we can merge it in to 3.10 (I think that makes more sense). I don't expect another 3.9 release (hopefully). It also introduces a new API endpoint so 3.10 is a better target.

avatar Fedik
Fedik - comment - 17 Jan 2021

would be cool if we have the double initialization added in this pr too

it is here, that why I made this fork:

// Set up only once
if ($target.data('showonInitialised')) {
return;
}
$target.data('showonInitialised', true);

I meant that for J4 will be need a separted PR.

It also introduces a new API endpoint so 3.10 is a better target.

I can remove Joomla.setUpShowon and leave "event" $(document).on('subform-row-add'), I guess most people who need it already use in this way, so it will be "bug fix" ?

avatar Fedik Fedik - change - 17 Jan 2021
Labels Added: ?
avatar Fedik Fedik - change - 17 Jan 2021
Title
Joomla.setUpShowon, fork of 18843
Joomla.Showon.initilise API, fork of 18843
avatar Fedik Fedik - edited - 17 Jan 2021
avatar Fedik
Fedik - comment - 17 Jan 2021

Okay, so, I have backport Joomla 4 'joomla:updated' stuff for showon, now it can be more easy for transition.
And added Joomla.Showon.initilise as @dgrammatiko suggested, later I will make pull for J4.

Testing the same, make sure showon still work.

Please someone change target branch to joomla-10, or I have to make new PR?

avatar Fedik Fedik - change - 17 Jan 2021
Title
Joomla.Showon.initilise API, fork of 18843
Joomla.Showon.initilise API, and make sure that showon initialised only once, fork of 18843
avatar Fedik Fedik - edited - 17 Jan 2021
avatar joomla-cms-bot joomla-cms-bot - change - 17 Jan 2021
Category JavaScript Layout Libraries External Library JavaScript Front End Plugins Unit Tests
avatar HLeithner
HLeithner - comment - 17 Jan 2021

I change the branch for you.

But you can do this your self by adding the PR
image

Thanks for your PR, now we need some tests ;-)

avatar Fedik
Fedik - comment - 17 Jan 2021

well, I just broke PR
need some time to fix

okay, I think I need to sync with j 3.10 branch

avatar Fedik
Fedik - comment - 17 Jan 2021

@HLeithner can I merge joomla:3.10-dev to my branch for sync? or I need make new

avatar HLeithner HLeithner - change - 17 Jan 2021
Labels Added: ? ?
avatar HLeithner
HLeithner - comment - 17 Jan 2021

I updated your branch you only need to pull from github again.

avatar HLeithner
HLeithner - comment - 17 Jan 2021

but yes you can simple merge 3.10-dev into your branch

avatar Fedik
Fedik - comment - 17 Jan 2021

okay, thanks

avatar richard67
richard67 - comment - 17 Jan 2021

@HLeithner Now the PR shows more changes than only those from @Fedik . It shows the changes which meanwhile have been made in the staging branch since the last upmerge of staging into 3-10-dev. So maybe it's easier just to redo the PR for 3.-10-dev?

avatar Fedik
Fedik - comment - 17 Jan 2021

@richard67 or we can wait when staging will be merged in to 3-10-dev :)

avatar Fedik Fedik - change - 17 Jan 2021
Title
Joomla.Showon.initilise API, and make sure that showon initialised only once, fork of 18843
[3.10] Joomla.Showon.initilise API, and make sure that showon initialised only once, fork of 18843
avatar Fedik Fedik - edited - 17 Jan 2021
avatar richard67
richard67 - comment - 17 Jan 2021

Yes, that will also work.

avatar HLeithner
HLeithner - comment - 17 Jan 2021

I will upmerge staging

avatar HLeithner HLeithner - change - 17 Jan 2021
Labels Added: ?
Removed: ?
avatar joomla-cms-bot joomla-cms-bot - change - 17 Jan 2021
Category JavaScript Layout Libraries External Library Front End Plugins Unit Tests JavaScript
avatar HLeithner
HLeithner - comment - 17 Jan 2021

so now it's ok again

avatar richard67
richard67 - comment - 17 Jan 2021

Yes, looks ok now.

avatar Fedik Fedik - change - 17 Jan 2021
Labels Removed: ?
avatar Fedik Fedik - change - 17 Jan 2021
The description was changed
avatar Fedik Fedik - edited - 17 Jan 2021
avatar Fedik
Fedik - comment - 17 Jan 2021

I have updated testing instruction, and made same changes for joomla 4 #32069

04e2d9d 17 Jan 2021 avatar Fedik typo
avatar Fedik Fedik - change - 17 Jan 2021
Title
[3.10] Joomla.Showon.initilise API, and make sure that showon initialised only once, fork of 18843
[3.10] Joomla.Showon.initialise API, and make sure that showon initialised only once, fork of 18843
avatar Fedik Fedik - edited - 17 Jan 2021
avatar dgrammatiko dgrammatiko - test_item - 17 Jan 2021 - Tested successfully
avatar dgrammatiko
dgrammatiko - comment - 17 Jan 2021

I have tested this item successfully on 04e2d9d


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

avatar gostn gostn - test_item - 22 Jan 2021 - Tested successfully
avatar gostn
gostn - comment - 22 Jan 2021

I have tested this item successfully on 04e2d9d


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

avatar richard67 richard67 - alter_testresult - 22 Jan 2021 - dgrammatiko: Tested successfully
avatar richard67 richard67 - alter_testresult - 22 Jan 2021 - gostn: Tested successfully
avatar richard67 richard67 - change - 22 Jan 2021
Status Pending Ready to Commit
avatar richard67 richard67 - edited - 22 Jan 2021
avatar richard67
richard67 - comment - 22 Jan 2021

RTC


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

avatar richard67 richard67 - change - 22 Jan 2021
Build staging 3.10-dev
avatar zero-24
zero-24 - comment - 22 Jan 2021

@Fedik do we need to add documentation for this change somewhere? And if so what should be documented?

avatar Fedik
Fedik - comment - 22 Jan 2021

@zero-24 good question, I have no idea ;)
Do we have any documentation about showon API somwhere? if no, then maybe we need to add.

Technically and partly it is a new feature, to cover edge cases from #18843 , also see comment #18843 (comment)

In J4 we already have events #16016 (I backported it partly here), that handle "dynamic" content changes

avatar zero-24
zero-24 - comment - 22 Jan 2021

Ok will merge than this heree for 3.10. thanks

avatar zero-24 zero-24 - change - 22 Jan 2021
Status Ready to Commit Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2021-01-22 19:26:36
Closed_By zero-24
Labels Added: ?
Removed: ?
avatar zero-24 zero-24 - close - 22 Jan 2021
avatar zero-24 zero-24 - merge - 22 Jan 2021

Add a Comment

Login with GitHub to post a comment