Arvados

Since clippy is now retired (#3177), we can update the script tag in collection show_files partial.

<script>
// The "each" loop in select_all_files() and unselect_all_files()
// is needed because .trigger("change") does not play well with clippy.
// Once clippy has been retired, we should be able to compress this
// into .filter(":visible").prop("checked", true).trigger("change").
...
</script>

• Please use 4525-remove-stale-clippy-code to review code updates.

• The branch 4525-remove-stale-clippy-code addressed the three items listed in description. It also addresses updating selection menu options when project tabs are switched (#4694)

• All workbench tests are passing

• Manual testing passed for the following:
  • Select one or more collections and expect the project selection options selected
  • select only one pipeline instance and expect that compare selection option is still disabled
  • select two or more pipeline instances and expect that compare selection option is enabled and compare operation works
  • Copy selected, move selected, combine selected worked as expected
  • In a Collection page: select all; unselect all; select one or more and then select all; deselect one or more and then click on unselect all; select all and unselect just one using the checkbox are all working after the uddate
  • In a collection page, combine selected files is working as expected
  • Add data to a project (using the selection in the search dialog) worked
  • Run a pipeline by selecting an input (in the input selection dialog) worked
  • Switch project tabs and observe that the selection options are reflecting only what is applicable to this tab

In apps/workbench/app/helpers/application_helper.rb I think the following lines provide a few recent items from get_n_objects_of_class, and aren't related to clippy:

-
-    if selectables.any?
-      lt += raw("add_form_selection_sources(#{selectables.to_json});\n")
-    end

I'm not keen on the ":visible" selector. Doesn't that omit stuff you selected on page 2 before scrolling up to the top to hit the action button, for example? Anyway, I think the real issue here was that enable_disable_selection_actions wasn't restricting itself to one $container at a time. I've pushed an alternate solution on 4525-remove-stale-clippy-code-TC @ 51aaccb. Thoughts?

Tom,

• I added back add_form_selection_sources in js and helper. I did not add back the function select_form_sources, which needs get_selection_list, which in turn needs storage and adding each selection to storage. Please confirm this is correct. My manual testing (using the above listed steps) worked. Also, all the tests passed.

• Merged your branch with updated enable_disable_selection_actions into my branch. All manual and test verifications passed. So, I think this is good.

Thanks.

Looked closer at the "selection sources" stuff. I think the current setup is:

• Build an array of options in selectables: [{name: "foo", uuid: "bar", type: "baz"}, ...]
• Use the inline script tag to pass selectables to add_form_selection_sources().
• Use the inline script tag to instruct X-editable to retrieve the selection dropdown content from select_form_sources().

• Combine the selectables with clippy stuff, and feed the combined set to X-editable when it calls select_form_sources.

I think that means all we need now is:

• Build an array of options in selectables -- but formatted the way x-editable wants it: [{text: "foo", value: "bar"}, ...]
• Put the array in "data-source" in the link_to attrtext, '#', ... call.
• No inline script tag.

• Remove all of the form_selection_sources stuff.

(update after IRC chat)

If the "selectables" stuff isn't used at all, we don't need "data-source" from my previous comment, and we can remove the whole # preload data block from application_helper.rb as well.

Peter said (IRC):

I think you want to get rid of
#preload data and the code and the "if" block and the <script> section
and keep the link_to

The link_to will be used when
attrvalue.is_a? Fixnum datatype = 'number' and
attrvalue.is_a? String datatype = 'text'
if you have a number or text you still need to render the editing control

LGTM (cdef2ed).

Now that #selection-form-content doesn't exist, I think we can also delete this from apps/workbench/app/assets/javascripts/selection.js.erb:

     $('#selection-form-content').on("click", function(e) {
         e.stopPropagation();
     });

...and the entire apps/workbench/app/assets/stylesheets/selection.css file.

Thanks Tom. I made those changes also.