? ? Success

User tests: Successful: Unsuccessful:

avatar brianteeman
brianteeman
5 Aug 2015

This PR is an attempt to clarify the copy/move issues users are reporting with the Batch processing and is based on an idea from Crystal

Before

5dk6

After

caf7

Testing

The div highlighted below is hidden until a category is selected - thanks @dgt41

hsdkjfs

Testing

Make sure that batch processing still works in all core components and menus and categories

avatar brianteeman brianteeman - open - 5 Aug 2015
avatar brianteeman brianteeman - change - 5 Aug 2015
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 5 Aug 2015
Labels Added: ? ?
avatar brianteeman brianteeman - change - 5 Aug 2015
Category Administration Language & Strings UI/UX
avatar brianteeman brianteeman - change - 5 Aug 2015
Easy No Yes
avatar infograf768
infograf768 - comment - 5 Aug 2015

I am not a fan of a dropdown with : - Don't move or copy - as title

avatar brianteeman
brianteeman - comment - 5 Aug 2015

Never expected you to agree.

avatar brianteeman
brianteeman - comment - 5 Aug 2015

Updated with the code from @dgt41 to hide the div

avatar jwaisner
jwaisner - comment - 5 Aug 2015

@test

Getting an error on move or copy of any articles:

screen shot 2015-08-05 at 18 06 41


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

avatar jwaisner jwaisner - test_item - 5 Aug 2015 - Tested unsuccessfully
avatar brianteeman
brianteeman - comment - 5 Aug 2015

I cant repeat that

On 6 August 2015 at 00:06, jwaisner notifications@github.com wrote:

@test https://github.com/test

Getting an error on move or copy of any articles:

[image: screen shot 2015-08-05 at 18 06 41]

https://camo.githubusercontent.com/d21dbeeabc30c08f4a32782dd2f0493c1c154a47/687474703a2f2f6973737565732e6a6f6f6d6c612e6f72672f75706c6f6164732f312f32613964376433303663386562326162363064666463383735646335383530662e706e67

This comment was created with the J!Tracker Application
https://github.com/joomla/jissues at issues.joomla.org/joomla-cms/7637
http://issues.joomla.org/tracker/joomla-cms/7637.


Reply to this email directly or view it on GitHub
#7637 (comment).

Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/

avatar jwaisner
jwaisner - comment - 6 Aug 2015

I tested again with a hard reset of all github files to clear out any possible anomalies, but I am still getting the same error when attempting to batch move articles, or copy the articles.


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

avatar dgt41
dgt41 - comment - 6 Aug 2015

@jwaisner what joomla version are you using? you should test this with the latest staging. Also I cannot replicate this

avatar jwaisner
jwaisner - comment - 6 Aug 2015

Ahh, I was using the Master Branch... I will need to retest tomorrow on staging.


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

avatar jwaisner jwaisner - test_item - 6 Aug 2015 - Not tested
avatar jwaisner
jwaisner - comment - 6 Aug 2015

@test

PR works as intended. Especially when you use the right branch :)


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

avatar jwaisner jwaisner - test_item - 6 Aug 2015 - Tested successfully
avatar jwaisner
jwaisner - comment - 6 Aug 2015

actually test has one error noted below in categories:

screen shot 2015-08-05 at 21 38 51


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

avatar jwaisner jwaisner - test_item - 6 Aug 2015 - Not tested
avatar brianteeman
brianteeman - comment - 6 Aug 2015

You are still not on staging that was an unrelated error which was fixed in
#7634

avatar brianteeman
brianteeman - comment - 6 Aug 2015

Updated with new code from @dgt41


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

avatar jwaisner
jwaisner - comment - 7 Aug 2015

Hmm.. Okay I will attempt to rebuild my setup this weekend. Appears something isnt right.


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

avatar jwaisner jwaisner - test_item - 7 Aug 2015 - Not tested
avatar jwaisner
jwaisner - comment - 7 Aug 2015

@test

After rebuilding my setup I can say that the PR successfully works on the Staging branch with no errors.


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

avatar jwaisner jwaisner - test_item - 7 Aug 2015 - Tested successfully
avatar Bakual Bakual - change - 7 Aug 2015
Milestone Added:
avatar Bakual Bakual - test_item - 7 Aug 2015 - Tested successfully
avatar Bakual
Bakual - comment - 7 Aug 2015

Well done! Test was successful with articles, banners and my own component.

avatar Bakual
Bakual - comment - 7 Aug 2015

One thing I wonder: Is it needed to change the following strings?

  • COM_CONTACT_BATCH_MENU_LABEL
  • COM_CONTENT_BATCH_MENU_LABEL
  • COM_NEWSFEEDS_BATCH_MENU_LABEL

The following is still in use, but could easily changed to use JLIB_HTML_BATCH_MENU_LABEL

  • COM_CATEGORIES_BATCH_CATEGORY_LABEL

The only one we still need is COM_MENUS_BATCH_MENU_LABEL because it has a reference to the menu in it.

That way translators don't have to translate the exact same string multiple times.

avatar Bakual
Bakual - comment - 7 Aug 2015

For reference: 3rd parties only need to remove the leading "COM_FOO_BATCH_TIP" to stay cinsistent with the core extensions. If they leave it there, no harm anyway.
All the rest is applied automatically.

avatar brianteeman
brianteeman - comment - 7 Aug 2015

My approach with the language strings is to always assume there was a good
reason it was done that way originally even if I could not see it. So if
there was a string per component I kept it that way. It might not make
sense in en-gb to have multiple strings but I don't know about other
languages so took the safe option. (We do this a lot throughout the
strings). It doesn't bother me if we keep or remove them.

avatar brianteeman
brianteeman - comment - 7 Aug 2015

Thanks for testing. If someone can make a decision on the strings we can either move to RTC or I can update them and move to RTC


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

avatar Bakual
Bakual - comment - 7 Aug 2015

The JLib strings were introduced with 4e6f0ee (JC http://joomlacode.org/gf/project/joomla/tracker/?action=TrackerItemEdit&tracker_item_id=22590).

I think the ones in the component were replaced with this, or were never used to begin with (copy-pasting stuff).
We could remove them, but we decided to not remove strings for B/C reasons.
I would just leave them alone. With J4 we will have a lot to clean up there :smile:

avatar Bakual
Bakual - comment - 7 Aug 2015

It would be great if we can replace the use of COM_CATEGORIES_BATCH_CATEGORY_LABEL with JLIB_HTML_BATCH_MENU_LABEL. What do you think?

avatar brianteeman
brianteeman - comment - 7 Aug 2015

If thats what you want I will do it. Cant see the point myself but I will
do what I am told?

On 7 August 2015 at 09:17, Thomas Hunziker notifications@github.com wrote:

It would be great if we can replace the use of
COM_CATEGORIES_BATCH_CATEGORY_LABEL with JLIB_HTML_BATCH_MENU_LABEL. What
do you think?


Reply to this email directly or view it on GitHub
#7637 (comment).

Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/

avatar Bakual
Bakual - comment - 7 Aug 2015

The point would be to reduce the amount of redundant strings we have, to make life easier for translators in the end.
It doesn't matter much. Would just be a small improvement I think :smile:

avatar infograf768
infograf768 - comment - 7 Aug 2015

+1 to create a new string "JLIB_HTML_BATCH_MENU_LABEL" as my tests show that it is translated the same in all languages.

Also, please do not forget weblinks in that case as the constant would change.

avatar Bakual
Bakual - comment - 7 Aug 2015

Merged. I did change the categories batch to use the string from jlib and also added deprecation comments to the unused strings.

avatar brianteeman brianteeman - change - 7 Aug 2015
Status Pending Closed
Closed_Date 0000-00-00 00:00:00 2015-08-07 10:14:52
Closed_By brianteeman
avatar brianteeman brianteeman - close - 7 Aug 2015
avatar brianteeman
brianteeman - comment - 7 Aug 2015

thanks - are we adding deprecated comments now on strings - we have hundreds

On 7 August 2015 at 11:13, Thomas Hunziker notifications@github.com wrote:

Merged. I did change the categories batch to use the string from jlib and
also added deprecation comments to the unused strings.


Reply to this email directly or view it on GitHub
#7637 (comment).

Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/

avatar brianteeman brianteeman - head_ref_deleted - 7 Aug 2015
avatar Bakual
Bakual - comment - 7 Aug 2015

Good question. I thought we occasionally did that already earlier, but now when I search for it I didn't find any.

Imho it could make sense especially when there is a replacement by a global one. Maybe not so much if it's just no longer in use.

I can revert it easily if it adds more confusion than it helps.

avatar zero-24
zero-24 - comment - 7 Aug 2015

IIRC We have a srcipt that can detect unused language strings (but i can't remember where it was)

avatar brianteeman
brianteeman - comment - 7 Aug 2015

The script is from @adaneinspain not 100% perfect as there are some false
positives

On 7 August 2015 at 11:27, zero-24 notifications@github.com wrote:

IIRC We have a srcipt that can detect unused language strings (but i can't
remember where it was)


Reply to this email directly or view it on GitHub
#7637 (comment).

Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/

avatar brianteeman
brianteeman - comment - 7 Aug 2015

Yes thats the one. Warning if using it do it one language file at a time and double check the unused strings it reports as it wont find combination strings

Add a Comment

Login with GitHub to post a comment