Following on the discussions in #3178648: [META] Support asset hierarchy...

Create a form at location/hierarchy/%parent that serves the same purpose that /admin/structure/taxonomy/farm_areas served in farmOS 1.x: a drag-and-drop UI for arranging the hierarchy of location assets ("area terms" in 1.x lingo).

Key requirements:

- Allow working with subsets of the hierarchy by including a /%parent parent ID argument in the URL.
- Allow modifying the asset parents (hierarchy) but not weight (always sort alphabetically)
- Handle assets with multiple parents intuitively (allow moving the asset under one parent without affecting the other(s))

CommentFileSizeAuthor
#4 3179023-4.patch12.58 KBpcambra
#4 farm_asset_reorder.zip5.5 KBpcambra

Issue fork farm-3179023

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

m.stenta created an issue. See original summary.

m.stenta’s picture

m.stenta
he/him
Primary language English
commented
Issue summary: View changes
pcambra’s picture

pcambra
he/him
Primary language Spanish
commented
Assigned: Unassigned » pcambra
pcambra’s picture

pcambra
he/him
Primary language Spanish
commented
Status: Active » Needs review
StatusFileSize
new farm_asset_reorder.zip5.5 KB
new 3179023-4.patch12.58 KB

Here's a first stub of the feature, integrating it back from the standalone module.
I haven't been able to test this bit because we need to integrate https://www.drupal.org/project/inspire_tree for composer to realise what it needs to do.

Attaching the standalone module too!

m.stenta’s picture

m.stenta
he/him
Primary language English
commented
Status: Needs review » Needs work

Thanks @pcambra! This is great!!!

I've started a 2.x-location-hierarchy branch with your patch as the first commit: https://github.com/mstenta/farmOS/tree/2.x-location-hierarchy - I figure we can work in this branch moving forward (feel free to check it out yourself and add commits).

I moved the relevant code out of the asset module and into a new farm_ui_location module. I also moved /asset-locations to simply /locations and added a top level menu item for it.

Lastly, I added $query->condition('is_location', TRUE); to the buildTree() method to limit the asset to "location assets".

That's as far as I got today. :-)

Here are some things that come to mind as next steps...

Only show /assets/%/locations tab when an asset is a location and has children.

Move /assets/%/locations to secondary tabs (like we did in the forest module).

I'm curious if we can make the hierarchy editing "togglable". In other words: when the page first loads, it is NOT draggable, but provides some kind of button or link to "modify hierarchy" which then enables the JS. This would enable the page to serve a dual-purpose: both for viewing the list of locations in a hierarchical manner, as well as modifying the hierarchy. If you look at the /farm/areas page in farmOS 1.x right now you'll notice that it serves the former purpose (viewing the hierarchy), but also has a "change hierarchy" link which takes you to the Drupal core taxonomy hierarchy edit form. By having a "view" mode, we can also make the location names into links that take you to the individual asset records. Thus, both goals can be achieved in the same page! What do you think?

Thinking ahead... I'm also imagining adding the farmOS map to the top of the /locations page. Then we'd be at 100% parity (and even better!) with the farmOS 1.x /farm/areas page. It would also be neat to explore adding a map to the /asset/%/locations page (filtered to only include the child assets), but that may be a bit trickier. Both of those can be done in follow-up - they don't need to block this.

m.stenta’s picture

m.stenta
he/him
Primary language English
commented

Another thought: I'm not sure the "ordered list" adds anything - and may just be confusing. The order doesn't get saved. Perhaps we could just change it to an unordered list?

It's also not super-obvious that the items are actually movable with click-and-drag. But perhaps just making the "toggle to hierarchy editor" step more intentional would fix that?

paul121’s picture

paul121 commented

This is great! TIL there is a Drupal.Message() JS API - awesome!

Perhaps we could just change it to an unordered list?

+1

Not sure how much is possible.. but maybe this could replicate parts of the Taxonomy drag and drop UI? The "drag" icon used there is nice. I also like that everything is shown in a table element, its a nice UI. This would be a nice way to add an additional column for the "asset type" field. Also replicating the "Operations" column with a dropbutton to view/edit/delete each location asset could be handy.

It's also not super-obvious that the items are actually movable with click-and-drag. But perhaps just making the "toggle to hierarchy editor" step more intentional would fix that?

I was confused by this too. Having the toggle would be great! But simply including some instructional text could go a long ways as well. Something to the effect of: "These are all of the assets with is_location enabled. The asset parent relationship is represented as the hierarchy." And when in editing mode... add another bit of text that explains "Drag and drop the asset names to modify the location hierarchy."

pcambra’s picture

pcambra
he/him
Primary language Spanish
commented
Related issues: +#3209164: Allow toggling between CDN/local JS hosting
pcambra’s picture

pcambra
he/him
Primary language Spanish
commented

Many thanks for the clear branch and feedback, folks!

Adding a toggle button that "enables" the edit of the hierarchy would be really good imho. As it could be a risk to move something by accident.

Regarding the ordered list, I've been looking at this and I think this is how the library works, we could add some CSS to remove the list style type and add some crosshair icon.

pcambra’s picture

pcambra
he/him
Primary language Spanish
commented

Remainingt tasks:

m.stenta’s picture

m.stenta
he/him
Primary language English
commented

Sorry to keep adding things but just found this in my notes... :-)

  • When the parent of an asset is changed, create an asset entity revision for that change.
m.stenta’s picture

m.stenta
he/him
Primary language English
commented

Move the "Locations" tab to secondary tabs

I'm having second thoughts about this - especially in lieu of recent thoughts/discussions around #3188586: Add Views of logs that reference assets.

Let's hold off on moving any tabs to the secondary tab level for now and think about all of them more holistically.

m.stenta’s picture

m.stenta
he/him
Primary language English
commented
Issue tags: +beta blocker
pcambra’s picture

pcambra
he/him
Primary language Spanish
commented

WIP is in https://github.com/pcambra/farmOS/tree/2.x-location-hierarchy

pcambra’s picture

pcambra
he/him
Primary language Spanish
commented
Related issues: +#3216766: Revisions are not supported on JSON API
pcambra’s picture

pcambra
he/him
Primary language Spanish
commented

Added PR on https://github.com/mstenta/farmOS/pull/1

pcambra’s picture

pcambra
he/him
Primary language Spanish
commented

- Sometimes can't drag child into another folder (parent) - we thought it had to do with the parent being expanded/not - but then it happened even when it was expanded too... seems inconsistent, hard to replicate

- Nice to have: disable link while drag+drop

- When item is moved to a new parent, in the /asset/%/locations page, it ADDs a parent but does not remove the original parent - resulting in 2 parents... but this does NOT happen in /locations. This is caused by the toggle not refreshing the tree properly.

  • m.stenta committed eba7ec0 on 2.x 
    Issue #3179023 by pcambra, m.stenta: Location hierarchy drag-and-drop...
m.stenta’s picture

m.stenta
he/him
Primary language English
commented
Status: Needs work » Fixed

This is awesome @pcambra! Merged!

Status: Fixed » Closed (fixed)

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