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

[NEW FEATURE] Adding Edit Associations Toolbar #21022

Merged
merged 13 commits into from Jul 16, 2018

Conversation

Projects
None yet
8 participants
@infograf768
Copy link
Member

infograf768 commented Jul 9, 2018

Pull Request for Issue #21003

Summary of Changes

Adding a new toolbar for items which can be associated.
The toolbar button redirects to the side by side Multilingual associations component.
Changes include new methods in FormController as in AdminModel.
And the new toolbar button for each view concerned.

Testing Instructions

Install a multilngual site. Make sure Associations are set.
Edit any

  • article
  • contact
  • menu item
  • newsfeed
  • category

Click on the new Edit Associations Toolbar button. Example:
screen shot 2018-07-09 at 11 57 31

You should be redirected to the side by side.
screen shot 2018-07-09 at 12 08 12

If the item is set to ALL languages, a Notice will display.
screen shot 2018-07-09 at 12 09 16

Documentation Changes Required

I guess so. Target is 3.9.x

@Bakual @rdeutz @alikon

@infograf768

This comment has been minimized.

Copy link
Member Author

infograf768 commented Jul 9, 2018

Looks like this needs some cs corrections to make drone happy...
Trying now.

infograf768 added some commits Jul 9, 2018

cs
cs

@infograf768 infograf768 added this to the Joomla 3.9.0 milestone Jul 9, 2018

}
else
{
if ($data['language'] === '*')

This comment has been minimized.

@Quy

Quy Jul 9, 2018

Contributor

Minor suggestion. Lines 1579-1596 don’t need to be in the else statement.

This comment has been minimized.

@infograf768

infograf768 Jul 9, 2018

Author Member

Indeed, will correct.

@@ -281,6 +281,7 @@ JGLOBAL_ARTICLE_ORDER_DESC="The order that articles will show in."
JGLOBAL_ARTICLE_ORDER_LABEL="Article Order"
JGLOBAL_ARTICLES="Articles"
JGLOBAL_ASSOC_NOT_POSSIBLE="To define associations, please make sure the item language is not set to 'All'."
JGLOBAL_ASSOCIATIONS_NEW_ITEM_WARNING="To edit associations, first save the item."

This comment has been minimized.

@brianteeman

brianteeman Jul 9, 2018

Contributor

==> To create an association, first save the item.

But can this actually be displayed as the button isnt presnt until you have saved the item

This comment has been minimized.

@infograf768

infograf768 Jul 9, 2018

Author Member

This is just a security warning in case something goes wrong. Will change string as indeed when not saved one can't edit any assoc.

@@ -924,6 +925,7 @@ JTOOLBAR_DELETE_ALL="Delete All"
JTOOLBAR_DISABLE="Disable"
JTOOLBAR_DUPLICATE="Duplicate"
JTOOLBAR_EDIT="Edit"
JTOOLBAR_EDIT_ASSOCIATIONS="Edit Associations"

This comment has been minimized.

@brianteeman

brianteeman Jul 9, 2018

Contributor

It is both edit and create so let's just say "Associations"

This comment has been minimized.

@infograf768

infograf768 Jul 9, 2018

Author Member

I have been wondering about that. The issue is that we already have an Associations Tab.

@brianteeman

This comment has been minimized.

Copy link
Contributor

brianteeman commented Jul 9, 2018

Other than my language string comments - this works well

@brianteeman

This comment has been minimized.

Copy link
Contributor

brianteeman commented Jul 9, 2018

The check to ensure that the language is not "all" doesnt check that it has actually been saved with a set language.

assoc

@infograf768

This comment has been minimized.

Copy link
Member Author

infograf768 commented Jul 9, 2018

hmm
this needs some thought. After siesta.

@brianteeman

This comment has been minimized.

Copy link
Contributor

brianteeman commented Jul 9, 2018

I have tested this item 🔴 unsuccessfully on f4e6698


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

@infograf768

This comment has been minimized.

Copy link
Member Author

infograf768 commented Jul 9, 2018

The issue is interesting as we have the same behavior when using the associations Tab (through js).
In that last case though, after choosing one or multiple associations we DO save the item, preventing this problem.
Let's see what we can do here.

@infograf768

This comment has been minimized.

Copy link
Member Author

infograf768 commented Jul 9, 2018

@brianteeman
Please test again. It works fine now here.

@infograf768

This comment has been minimized.

Copy link
Member Author

infograf768 commented Jul 9, 2018

Note: the multilingual manager may not show the item once target is saved and Close button has been used. But the item IS saved with the selected language and correctly associated to the target. Just select the right type of item and language.
One way to see that is to look at the Associations column in the mulilingual manager or the item manager itself (whatever item concerned).

@micker

This comment has been minimized.

Copy link

micker commented Jul 9, 2018

same return, in fact that a great addition ! we win a lot of time thanks !

@infograf768

This comment has been minimized.

Copy link
Member Author

infograf768 commented Jul 9, 2018

@infograf768

This comment has been minimized.

Copy link
Member Author

infograf768 commented Jul 10, 2018

@brianteeman
The issue you described in #21022 (comment) is now solved.
Please test again.
edit_assoc

@micker

This comment has been minimized.

Copy link

micker commented Jul 10, 2018

Tested its ok for me


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

@franz-wohlkoenig

This comment has been minimized.

Copy link

franz-wohlkoenig commented Jul 10, 2018

at Issue Tracker Test of @micker is shown as successfully. I altered it too but it isn't shown here.

@infograf768

This comment has been minimized.

Copy link
Member Author

infograf768 commented Jul 10, 2018

at Issue Tracker Test of @micker is shown as successfully. I altered it too but it isn't shown here.

Yes, I had marked it as successful as he did not do it really, but we have an issue.
We also have an issue with Travis which was working Ok before.

@brianteeman

This comment has been minimized.

Copy link
Contributor

brianteeman commented Jul 10, 2018

The issue I reported has been resolved.

BUT I found another one when "editing" associations.

When I open an item that is already associated and then select "edit associations" I expected to be redirected to com_associations with the item AND its association open but it is not.

Thinking about solving this I can see that on a bilingual site what I described would be the expected behaviour but it wouldnt be possible on a site with more than two languages.

So it can only be resolved, imho, by changing the expectation of clicking on the button "edit associations". Perhaps just saying "associations" on the button will do that.

infograf768 added some commits Jul 10, 2018

cs
@infograf768

This comment has been minimized.

Copy link
Member Author

infograf768 commented Jul 10, 2018

@brianteeman
I have improved this code IF we have only 2 Content Languages.
What will happen is the following:

  1. It will load directly an eventual associated item in the side by side.
  2. If no association it will already select the target language in the side by side.

It is obviously impossible to do the same if we have more than 2 content languages

@micker
This has to be tested again with this specific case.

@brianteeman

This comment has been minimized.

Copy link
Contributor

brianteeman commented Jul 10, 2018

@infograf768 awesome. If not already merged I will test tomorrow

infograf768 added some commits Jul 10, 2018

$lang_code[] = $language->lang_code;
}
$refLang = array($data['language']);

This comment has been minimized.

@Quy

Quy Jul 10, 2018

Contributor

Remove spaces before =

This comment has been minimized.

@infograf768

infograf768 Jul 11, 2018

Author Member

done. thanks.

cs
@jreys

This comment has been minimized.

Copy link
Contributor

jreys commented Jul 11, 2018

I have tested this item successfully on 0db5e28

Works as expected now :)


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

@infograf768

This comment has been minimized.

Copy link
Member Author

infograf768 commented Jul 12, 2018

@jreys Thanks for testing.

Needs one more test. @micker @brianteeman

@micker

This comment has been minimized.

Copy link

micker commented Jul 12, 2018

tested successfully on 0db5e28


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

@joomla-cms-bot joomla-cms-bot removed this from the Joomla 3.9.0 milestone Jul 12, 2018

@franz-wohlkoenig

This comment has been minimized.

Copy link

franz-wohlkoenig commented Jul 12, 2018

Ready to Commit after two successful tests.

@joomla-cms-bot joomla-cms-bot added the RTC label Jul 12, 2018

@franz-wohlkoenig

This comment has been minimized.

Copy link

franz-wohlkoenig commented Jul 12, 2018

why was successfully Test of @micker not shown at Issue Tracker (have altered Test now)?

@micker

This comment has been minimized.

Copy link

micker commented Jul 12, 2018

@franz-wohlkoenig sorry i am a newbie on tracker how i can do this

@franz-wohlkoenig

This comment has been minimized.

Copy link

franz-wohlkoenig commented Jul 12, 2018

@micker this was not to you, it was a Question on People knowing more than me how this could be (as similar happen a few Days ago).

On your Question as Example of this PR:

Please mark your Test as successfully:

  • open Issue Tracker
  • Login with your github-Account
  • Click on blue "Test this"-Button above Authors-Picture
  • mark your Test as successfully
  • hit "submit test result"
@franz-wohlkoenig

This comment has been minimized.

Copy link

franz-wohlkoenig commented Jul 12, 2018

@micker before i forget: Thanks for your Work.

@mbabker mbabker changed the base branch from staging to 3.9-dev Jul 16, 2018

@mbabker mbabker added PR-3.9-dev and removed PR-staging labels Jul 16, 2018

@mbabker mbabker added this to the Joomla 3.9.0 milestone Jul 16, 2018

@mbabker mbabker merged commit 2073b2b into joomla:3.9-dev Jul 16, 2018

4 of 5 checks passed

continuous-integration/appveyor/pr AppVeyor build failed
Details
Hound No violations found. Woof!
JTracker/HumanTestResults Human Test Results: 2 Successful 0 Failed.
Details
continuous-integration/drone/pr the build was successful
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@joomla-cms-bot joomla-cms-bot removed the RTC label Jul 16, 2018

@infograf768 infograf768 deleted the infograf768:edit_assoc_toolbar branch Jul 17, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.