Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[4.0] [com_associations] Fix language association "clear" action #27159

Merged
merged 7 commits into from
Dec 8, 2019

Conversation

Fedik
Copy link
Member

@Fedik Fedik commented Nov 26, 2019

Summary of Changes

The fix for issue described in #27111 by @infograf768

The Clear button is not acting as it correctly does in 3.x.
It should not only clear the existing association itself in the Reference but also reload the Target with empty item fields.

Testing Instructions

apply patch, run npm install
go to edit an existing language association, and push a clear btn

Expected result

works

Actual result

not works

@infograf768
Copy link
Member

@Fedik
First I merged #27111 as it contains a patch for administrator/components/com_associations/Field/Modal/AssociationField.php (wrong name for the js file)
Almost there. Clear works OK
Remains a problem when we use Copy Reference to Target after Clearing twice.
The reference now displays the last target created and the Iframe is blocked.

problem_sidebyside

@Fedik
Copy link
Member Author

Fedik commented Nov 27, 2019

hm, that also strange
need to look

@Fedik
Copy link
Member Author

Fedik commented Nov 30, 2019

@infograf768 I tried 8-10 times in row, to "copy to reference" and "clear", and it work without any blocking to me.
In theory it may happen if there a js error, can you please look in Browser dev console, is there any? (turn joomla to debug mode "on", so we will see a real line number for error)

@infograf768
Copy link
Member

@Fedik
I can't indeed totally replicate it. Debug console shows no error but look carefully at this capture:
The new target is first briefly loaded in reference before being replaced by the original reference with an iframe reload.

sidebyside_display2

The difference with my test before is that it was remaining stuck as reference and is now replaced.

Shall we just hope that it will remain working this weird way or try to figure out what's really going on?

@Fedik
Copy link
Member Author

Fedik commented Dec 2, 2019

@infograf768 I see you have 3 languages, and I have only 2, maybe somehow related also,
I will try to check more

@Fedik
Copy link
Member Author

Fedik commented Dec 6, 2019

@infograf768 I have tried one more time.
Sorry I still cannot replicate it

Shall we just hope that it will remain working this weird way or try to figure out what's really going on?

I think, we can test the patch as is now,
And if it will be a bug for multiple users, then we may get more info about that weird bug.

@infograf768
Copy link
Member

I have tested this item ✅ successfully on 722df45

Agree. Thanks. Let’s get this in.


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

@alikon
Copy link
Contributor

alikon commented Dec 8, 2019

I have tested this item ✅ successfully on d512b75


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

@alikon
Copy link
Contributor

alikon commented Dec 8, 2019

RTC


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

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Dec 8, 2019
@infograf768 infograf768 merged commit 808f25e into joomla:4.0-dev Dec 8, 2019
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Dec 8, 2019
@infograf768
Copy link
Member

Thanks.

@infograf768
Copy link
Member

@alikon
Have you remarked when testing the weird behavior I saw?

@infograf768 infograf768 added this to the Joomla 4.0 milestone Dec 8, 2019
@Fedik Fedik deleted the lang-assoc-fix branch December 8, 2019 12:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NPM Resource Changed This Pull Request can't be tested by Patchtester
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants