? Pending

User tests: Successful: Unsuccessful:

avatar C-Lodder
C-Lodder
21 Apr 2017

Pull Request for Issue # .

Summary of Changes

Start rewritting some of the jQuery in vanilla JS. I'm getting errors when trying to install a language so I have not been able to test any of this.

I'm not going to event attempt to rewrite sidebyside.js as it's an absolute cluster***k

Testing Instructions

Test com_assosiations and ensure it works as expected without any console log errors.

@dgt41

avatar C-Lodder C-Lodder - open - 21 Apr 2017
avatar C-Lodder C-Lodder - change - 21 Apr 2017
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 21 Apr 2017
Category JavaScript
avatar C-Lodder C-Lodder - change - 21 Apr 2017
Labels Added: ?
avatar infograf768
infograf768 - comment - 21 Apr 2017
avatar C-Lodder
C-Lodder - comment - 21 Apr 2017

@infograf768 - already tried that and getting a JS error from $.ajax on the installer

avatar infograf768
infograf768 - comment - 21 Apr 2017

will test again later on. It was working OK a few days ago.

avatar infograf768
infograf768 - comment - 21 Apr 2017

No problem here to install lang. Will now test this PR

avatar infograf768
infograf768 - comment - 21 Apr 2017

Works. Does not solve #15439

avatar dgt41
dgt41 - comment - 21 Apr 2017

@infograf768 this PR just removes a totally unneeded dependency (jQuery), the other issue is due to multiselect

avatar infograf768
infograf768 - comment - 21 Apr 2017

There is a difference though with 3.7.0 (before and after this PR)
When one loads an existing associated item, The Copy Reference to target is proposed in the side by side, which is not the case in 3.7.0 (3.7.0 is correct).
Basically Copy Reference to Target should only display and be used when the target is a new empty item.

Example:
screen shot 2017-04-21 at 16 59 59

instead of
screen shot 2017-04-21 at 17 01 41

avatar infograf768
infograf768 - comment - 21 Apr 2017

I have displayed above on top what is right : 3.7.0 : NO Copy ref to target
below what is wrong: 4.0 Copy to Target is present
In both cases for items which alre already associated

as any one can see at looking at the screenshots
Suggest ? ?

avatar C-Lodder
C-Lodder - comment - 21 Apr 2017

Sorry, just removed my comment as I just realised the 2nd screenshot was Joomla 4 :)

As for the button in the toolbar, I don't believe that has anything to do with this PR. It may need to be fixed, but still unrelated.

avatar infograf768
infograf768 - comment - 21 Apr 2017

The code to display or not the Copy is in the sidebyside.js and DOES need to be fixed (why does'nt it work in 4.0 is a mystery to me)
I has indeed apparently no relation with this PR. Therefore this can be merged.

avatar C-Lodder
C-Lodder - comment - 21 Apr 2017

Quote from my initial PR comment:

I'm not going to event attempt to rewrite sidebyside.js as it's an absolute cluster***k

avatar dgt41
dgt41 - comment - 21 Apr 2017

@infograf768 javascript is tightly connected to the markup, and that is slightly different is J4

avatar infograf768
infograf768 - comment - 21 Apr 2017

I understand that. I also saw that the Save Target button is displayed even when target is not even displayed yet.
we really need to solve these .

avatar infograf768
infograf768 - comment - 21 Apr 2017

Yeah, the markup indeed...
in 3.7.0 we have a

<div id="toolbar-target" class="btn-wrapper" style="display: inline-block;">
     <button class="btn btn-small btn-success" onclick="Joomla.submitbutton('target')">
etc.

in 4.0 a direct
<button class="btn btn-small btn-success" onclick="Joomla.submitbutton('target')">
No specific id for the button...
can't work with

jQuery(document).ready(function($) {
	$('#toolbar-target').hide();
	$('#toolbar-copy').hide();
avatar infograf768
infograf768 - comment - 21 Apr 2017

We do need ids for these toolbar buttons.
Can't we revert to 3.7.0 and re-add the

<div class="btn-wrapper" <?php echo $displayData['id']; ?>>
	<?php echo $displayData['action']; ?>
</div>

in toolbar/ base.php ?

Change btn-wrapper to whatever one likes.
Any issue with that?

avatar C-Lodder
C-Lodder - comment - 21 Apr 2017

The toolbar needs some improvements, which I'm no doubt going to have to do myself at some point and I'll also address this issue you've mentioned.

Could you create a new issue for that test this PR for what it does :) Let me know if there are any other errors and I'll fix them, as I've got the language to install properly now

avatar infograf768 infograf768 - close - 22 Apr 2017
avatar infograf768 infograf768 - merge - 22 Apr 2017
avatar infograf768 infograf768 - change - 22 Apr 2017
Status Pending Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2017-04-22 09:13:10
Closed_By infograf768
avatar infograf768
infograf768 - comment - 22 Apr 2017

As I already said, this PR is fine for me for what it does. Merged.

Add a Comment

Login with GitHub to post a comment