Skip to content

[6.0] Batch Copy & Move tags #41927

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

Open
wants to merge 47 commits into
base: 6.0-dev
Choose a base branch
from

Conversation

beni71
Copy link
Contributor

@beni71 beni71 commented Sep 26, 2023

Pull Request for Issue #23304, only partially.
This PR is based on #38583 and fixes the comments.

Summary of Changes

Adds the ability to batch copy/move tags in the tags list.
Copying a tag also copies its children (same behavior as in other tree models like categories, menues).
Moving a tag also moves its children (same behavior as in other tree models like categories, menues).
For example, it is possible to move a child tag from one parent to another (including its children), or to copy a child tag from one parent to another (including its children).
In the batch tag drop down there is also an item "Root" to move/copy selected tags to the root level.

Changed visualization "Action to Perform" of copy/move action, it is now hidden as long as the user does not touch the select field for copy/move. See example in video below. This change affects batch copy/move dialog of menues and articles as well.

It also fixes a browser console error when the batch dialog is opened and cancel is clicked, on systems where multilingual is disabled.

Testing Instructions

It contains an updated javascript so run: npm run build:js

Initial tag structure for each of the tests below:
Tag1
-Tag2
--Tag3
Tag4

Test 1
Move Tag2 to Tag4 results in:
Tag1
Tag4
-Tag2
--Tag3

Test 2
Move Tag1 to Tag4 results in:
Tag4
-Tag1
--Tag2
---Tag3

Test 3
Copy Tag4 to Tag2 result in:
Tag1
-Tag2
--Tag3
--Tag4 (2)
Tag4

Test 4
Check whether the visualization of "Action to Perform" of copy/move action is first hidden for views like menues, articles. Only if a target item is selected "Action to Perform" appears.

Actual result BEFORE applying this Pull Request

The tags list batch processing does not support copy move.

Expected result AFTER applying this Pull Request

The tags list batch processing supports copy move:

tag-copy-move

Link to documentations

Please select:

  • Documentation link for docs.joomla.org:

  • No documentation changes for docs.joomla.org needed

  • Pull Request link for manual.joomla.org:

  • No documentation changes for manual.joomla.org needed

@joomla-cms-bot joomla-cms-bot added Language Change This is for Translators NPM Resource Changed This Pull Request can't be tested by Patchtester PR-5.0-dev labels Sep 26, 2023
@beni71 beni71 marked this pull request as ready for review September 26, 2023 07:33
@ceford
Copy link
Contributor

ceford commented Sep 26, 2023

I have tested this item ✅ successfully on c503b03

Easy to get into a muddle but works as described.


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

@HLeithner HLeithner changed the title [5.0] Batch Copy & Move tags [5.1] Batch Copy & Move tags Sep 26, 2023
@chmst
Copy link
Contributor

chmst commented Sep 26, 2023

Thanks a lot for working on this issue! Unfortunately I am not in favour of this implementation, from UX point of view.
Batch copy and move is not quite easy to understand, but at least the process should be everywhere the same. Behavoiur must be predictable.
Would it be possible to implement this behaviour "as always"?

Always:

user selects an item in the list and activates the batch button
(the system has the ids of the selected items)

on the modal: the users selects a target for his copy or move from the dropdown

decides if the action copy or move is to be performed.

your implementation

user selects an item in the list and activates the batch button (as he uses to do in other places)
(the system ignores the selection)

grafik
The "keep original tag" is misleading. The user's selection in the list is ignored, he must select the items now in the dropdown list.

After the selection of an item from the list, the select is replaced by another question. This change is so fast and the texts look similiar that it is difficult to notice the replacement. The user could think that he still sees his selection.

grafik

It would be clearer if the new block is appended to the first one.

The process then works like a charm.

I think that this causes confusion

In my opinion, the process should be the same in all categories and all other components, then it does not need extra documentation.

@brianteeman
Copy link
Contributor

@chmst isnt this identical to the ui for batch copying and moving menu items??

@chmst
Copy link
Contributor

chmst commented Sep 26, 2023

@brianteeman not if I understand this correctly

You choose a menuitem in the list (the systems holds the id), opens the batch modal

grafik

then you choose a target menuitem and decides if you want a copy or move.

You must not do this first step and select your item in the batch modal.

@brianteeman
Copy link
Contributor

image

image

image

image

I dont see the difference

I think the confusion is because the language string is wrong
it should not be Keep Original Tags
it should be Dont Copy or move

image

@chmst
Copy link
Contributor

chmst commented Sep 27, 2023

Yes, thanks for explainingt. The language string is the first reason for confusion. But it must be "select a tag". Just open the dropdown.

The second is: Why not accept the selection of items to move or copy from the overview? In com_menus and in categories we don't have an extra step for that. Maybe there is something special in tags which requires that?

@brianteeman
Copy link
Contributor

@chmst Sorry I dont understand what you are talking about

@chmst
Copy link
Contributor

chmst commented Sep 27, 2023

I am sorry that I cannot explain. Will try it later with videos.

@chmst
Copy link
Contributor

chmst commented Sep 27, 2023

@brianteeman you were correct. - it seems that this language string caused the whole confusion and my folloing action was overreaction.
So it will be an easy fix. Thanks for your patience.

@beni71
Copy link
Contributor Author

beni71 commented Sep 27, 2023

Sorry for that confusion. I fixed the text and updated the description part of the PR.

@HLeithner HLeithner changed the base branch from 5.0-dev to 5.1-dev September 30, 2023 22:48
@HLeithner
Copy link
Member

This pull request has been automatically rebased to 5.1-dev.

@HLeithner HLeithner changed the base branch from 5.2-dev to 5.3-dev September 2, 2024 08:52
@HLeithner
Copy link
Member

This pull request has been automatically rebased to 5.3-dev.

@HLeithner HLeithner changed the title [5.2] Batch Copy & Move tags [5.3] Batch Copy & Move tags Sep 2, 2024
@Hackwar Hackwar removed the PR-5.2-dev label Sep 3, 2024
@rdeutz rdeutz removed the b/c break This item changes the behavior in an incompatible why. HEADS UP label Oct 29, 2024
@web54
Copy link

web54 commented Feb 22, 2025

I have tested this item ✅ successfully on 715e848

I tested it succesfully during PBF 2025


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

@HLeithner HLeithner changed the base branch from 5.3-dev to 6.0-dev March 4, 2025 17:21
@HLeithner
Copy link
Member

This pull request has been automatically rebased to 6.0-dev.

@HLeithner HLeithner changed the title [5.3] Batch Copy & Move tags [6.0] Batch Copy & Move tags Mar 4, 2025
@rdeutz rdeutz removed the PR-5.3-dev label Mar 5, 2025
@richard67 richard67 removed the PBF Pizza, Bugs and Fun label Aug 22, 2025
@richard67
Copy link
Member

@Hackwar Maybe you can advice with the PHPstan errors for this PR: One can be fixed with $this->getDocument(), all the others are all related to the deprecated getError() method. What should be done with them? Change this PR's error handling to catch exceptions? Or add them to the PHPstan baseline with this PR? If we get the PR fixed we can add back the "PBF" label.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature Language Change This is for Translators NPM Resource Changed This Pull Request can't be tested by Patchtester PR-6.0-dev
Projects
None yet
Development

Successfully merging this pull request may close these issues.