Feature/batch edit media #1101
Feature/batch edit media #1101
Conversation
Daniel-KM
force-pushed
the
feature/batch_edit_media
branch
2 times, most recently
from
bd3cce3
to
614bc64
Compare
|
Rebased. |
Daniel-KM
force-pushed
the
feature/batch_edit_media
branch
3 times, most recently
from
99ed8a3
to
e94230f
Compare
|
Rebased. |
Daniel-KM
force-pushed
the
feature/batch_edit_media
branch
from
e94230f
to
be7a656
Compare
|
Rebased. |
Daniel-KM
force-pushed
the
feature/batch_edit_media
branch
from
be7a656
to
bbfd21b
Compare
| continue; | ||
| } | ||
| $collectionAction = $this->has($key) | ||
| ? $this->get($key)->getAttribute('data-collection-action') |
There was a problem hiding this comment.
I'm not sure of the purpose of data-collection-action here. It's not anywhere else in the code.
There was a problem hiding this comment.
The main point of data-collection-action is to manage remove/replace/append choice automatically for modules.
| @@ -31,19 +33,56 @@ $sortHeadings = [ | |||
| <?php echo $this->sortSelector($sortHeadings); ?> | |||
| </div> | |||
|
|
|||
| <form method="post" id="batch-form" class="disable-unsaved-warning"> | |||
|
|
|||
| <div id="page-actions"> | |||
There was a problem hiding this comment.
I believe this div should be class="batch-inputs" not id="page-actions"
There was a problem hiding this comment.
The initial pr was six month ago... Some water has flowed under the bridges. I fixed it with the new commits.
| 'options' => [ | ||
| 'label' => 'Set visibility', // @translate | ||
| 'value_options' => [ | ||
| '' => '[No change]', // @translate |
There was a problem hiding this comment.
"[No change]" should be set as default.
| 'options' => [ | ||
| 'label' => 'Set openness', // @translate | ||
| 'value_options' => [ | ||
| '' => '[No change]', // @translate |
There was a problem hiding this comment.
"[No change]" should be set as default.
| } | ||
|
|
||
| // Set remaining elements according to attribute data-collection-action. | ||
| $processeds = [ |
There was a problem hiding this comment.
Why does this array include values already included as "remove" in the code above?
There was a problem hiding this comment.
See first comment.
| * @return array Associative array of data to replace, to remove and to | ||
| * append. | ||
| */ | ||
| public function preprocessData() |
There was a problem hiding this comment.
I agree with the logic behind moving this code from the disparate controllers to here. Perhaps it would be better suited as an input filter, but this is workable if that's too much trouble.
Daniel-KM
force-pushed
the
feature/batch_edit_media
branch
2 times, most recently
from
e120beb
to
8336e2a
Compare
|
Thanks! Did you pass #1100 too? |
|
The rebasing and so on is confusing Github but yes, I believe we already have all of #1100 by virtue of those changes being part of this PR also. |
Based on the previous pull request, a page to batch edit media like items and item sets.