Problem/Motivation

Once #3293014: Support multiple source plugins to be used simultaneously is merged, we should follow up with the ability to control the order of the tabs.

We should be able to control the enabled/disabled status of each plugin as well as the order within the admin screen.

Steps to reproduce

Enable multiple plugins, you can't control the order of them.

Proposed resolution

(Updated from suggestions in comments) Open decision to whoever tries to implement it first. Consider tabledrag.
* Maybe add drag & drop to the checkboxes.
* An alternative to checkboxes would be a disabled section, like Field UI.

Remaining tasks

  • ✅ File an issue about this project
  • ☐ Manual Testing
  • ☐ Code Review
  • ☐ Accessibility Review
  • ☐ Automated tests needed/written?
CommentFileSizeAuthor
#34 Screenshot from 2022-08-17 10-48-40.png77.38 KBvighneshh
#34 Screenshot from 2022-08-17 10-49-03.png58.95 KBvighneshh
#24 3299029 After MR .png60.72 KBaarti zikre
#24 3299029 After mr extends.png141.08 KBaarti zikre

Issue fork project_browser-3299029

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

fjgarlin created an issue. See original summary.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Priority: Normal » Major
Issue tags: +Project Browser 2.0

Will this only handle order, or will it also include enable/disable?

fjgarlin’s picture

fjgarlin commented

Good point, my idea was to handle both.

fjgarlin’s picture

fjgarlin commented
Issue summary: View changes

Updated description with more details.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented

I leave the decision to whoever tries to implement it first (tabledrag is a beast), but an alternative to checkboxes would be a disabled section, like Field UI. I'm fine with either.

fjgarlin’s picture

fjgarlin commented
Issue summary: View changes
narendrar’s picture

narendrar
He/Him
Primary language English
Location Jaipur, India
commented
Assigned: Unassigned » narendrar

narendraR opened merge request !220

narendrar’s picture

narendrar
He/Him
Primary language English
Location Jaipur, India
commented
Assigned: narendrar » Unassigned
Status: Active » Needs review
fjgarlin’s picture

fjgarlin commented
Assigned: Unassigned » narendrar
Status: Needs review » Needs work

I tested this on drupalpod. Drag and drop works as expected, but if I click on the dropdown to change region, I get a javascript error and it does not work.

Javascript error:

Uncaught Drupal.AjaxError
* message: "\nAn AJAX HTTP error occurred.\nHTTP Result Code: 200\nDebugging information follows.\nPath: /admin/config/development/project_browser?ajax_form=1\nStatusText: parsererror\nResponseText: "
* name: "AjaxError"

Watchdog entry:

TypeError: call_user_func_array(): Argument #1 ($callback) must be a valid callback, class Drupal\project_browser\Form\SettingsForm does not have a method "multistepSubmit" in call_user_func_array() (line 114 of /var/www/html/web/core/lib/Drupal/Core/Form/FormSubmitter.php)
narendrar’s picture

narendrar
He/Him
Primary language English
Location Jaipur, India
commented
Assigned: narendrar » Unassigned
Status: Needs work » Needs review

Thanks @fjgarlin for the review. Fixed the error in latest commit.

fjgarlin’s picture

fjgarlin commented
Version: 1.0.0-beta1 » 1.0.x-dev
Status: Needs review » Reviewed & tested by the community

I re-tested everything again and it works great! Easy and familiar UI. The code looks good too, and there are additional tests.

Marking RTBC though reminding the above note

Note: tabledrag is tricky, and I suggested to @NarendraR that he copy/paste from Field UI as a working example, and that we'd refactor from there. Just a heads up in case this concerned anyone, it won't be there by the time we merge this 😄
tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Status: Reviewed & tested by the community » Needs work

Oh while this is a great start, I'd like to improve the MR a bit more before merging (and avoid even a temporary dependency on Field UI).

@narendraR if you want to work on it more, please do. But if not, let me know and I'll try to make some time for it.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented

Those are out of order, but that's my suggestion on how to proceed next. Steps 1-6

narendrar’s picture

narendrar
He/Him
Primary language English
Location Jaipur, India
commented
Assigned: Unassigned » narendrar

tim.plunkett opened merge request !226

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented

Turns out the the magic part of tabledrag I was thinking of isn't part of tabledrag. Field UI, Block UI, and Views UI all have custom JS written to link together the select element and the tabledrag placement.

See block.es6.js: https://git.drupalcode.org/project/drupal/-/blob/9.5.x/core/modules/bloc...

I pushed up another branch for now with the rest of the changes, but we'll need our own version of tableDrag.onDrop in a custom (non-Svelte) JS.

narendrar’s picture

narendrar
He/Him
Primary language English
Location Jaipur, India
commented

block.js added and modified as per requirement to make drag/drop workable. But still lots of cleanup needs to be done in new js file and select list is still not working.
Also order of saved plugins is correct but it is not displayed correctly in front-end.

narendraR closed merge request !220

narendrar’s picture

narendrar
He/Him
Primary language English
Location Jaipur, India
commented
Assigned: narendrar » Unassigned
Status: Needs work » Needs review

bnjmnm made their first commit to this issue’s fork.

bnjmnm’s picture

bnjmnm
he/him
Primary language English
Location Ann Arbor, MI
commented

EDIT: -- comment left in wrong issue --

aarti zikre’s picture

aarti zikre commented
Assigned: Unassigned » aarti zikre

Reviewing this issue

aarti zikre’s picture

aarti zikre commented
StatusFileSize
new 3299029 After mr extends.png141.08 KB
new 3299029 After MR .png60.72 KB

Review the MR. Looks good to me.

We can change the order of the plugins. Also enable and disable feature is working properly(Attaching SS for reference).
However, Having some doubt.
Suppose currently, the sequence is
1. Drupal.org (mocked)
2. Random data

Then in the extend it shows the result of Drupal.org (mocked).
But if we change the sequence and make Random data at first position then still it shows the data of Drupal.org (mocked). So not sure on which bases it filtering data Based on the order or based on total result.

2022-08-12/3299029 After MR .png
2022-08-12/3299029 After mr extends.png

aarti zikre’s picture

aarti zikre commented
Assigned: aarti zikre » Unassigned
fjgarlin’s picture

fjgarlin commented

It's just storing in the cookies or local storage somewhere which one last the last one that you had open. I think it's a non-issue.

aarti zikre’s picture

aarti zikre commented
Status: Needs review » Reviewed & tested by the community

Okay than look good to me
Moving to RTBC

bnjmnm’s picture

bnjmnm
he/him
Primary language English
Location Ann Arbor, MI
commented
Status: Reviewed & tested by the community » Needs work

Left some feedback in the MR

narendrar’s picture

narendrar
He/Him
Primary language English
Location Jaipur, India
commented
Status: Needs work » Needs review

Resolved all threads but not able to mark them as resolved.

bnjmnm’s picture

bnjmnm
he/him
Primary language English
Location Ann Arbor, MI
commented
Status: Needs review » Needs work

I spotted one place that likely needs a bit more test coverage, but this is very close to done. Nice work @narendraR

narendrar’s picture

narendrar
He/Him
Primary language English
Location Jaipur, India
commented
Status: Needs work » Needs review
vighneshh’s picture

vighneshh commented
Assigned: Unassigned » vighneshh

i Will Review It.

vighneshh’s picture

vighneshh commented
Assigned: vighneshh » Unassigned
Status: Needs review » Reviewed & tested by the community

Done UI testing of this thing and everything works properly.
We can control the order of the plugin.

vighneshh’s picture

vighneshh commented
StatusFileSize
new Screenshot from 2022-08-17 10-49-03.png58.95 KB
new Screenshot from 2022-08-17 10-48-40.png77.38 KB

Added some screenshots for reference after the patch is applied.

  • tim.plunkett committed c3cbdd8 on 1.0.x 
    Issue #3299029 by narendraR, tim.plunkett, fjgarlin, bnjmnm: Control...
tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Status: Reviewed & tested by the community » Fixed

Great work, thanks!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.