benetech / Imageshare

MIT License
0 stars 0 forks source link

Group resource files by type #250

Closed jkva closed 3 years ago

jkva commented 3 years ago

When displaying resource files for a resource, group them by type. (Image, video, 3d model...)

jkva commented 3 years ago

@clapierre how are you seeing this? Only group when there's more than one file, and more than one type of file?

clapierre commented 3 years ago

Hi @jkva,

See below for what I was thinking.

Right, if we don't have more than one file for a group it wouldn't make sense to group them. I think this could be done in the JSON file as well where we wouldn't create groups if those fields aren't populated.

Just like we have the "resource_unique_name" where we group the resource files to a particular resource we will have a new "resource_unique_group_name" and a "resource_group_image_URI". So if these exist a group will be created with that name and that featured image for that group. If they don't exist it just gets added / views like we have now.

The Frog resource is a perfect example.

In this resource we would have 8 main groups Frog Dissection (print, color, labels) Frog Dissection (print, color, no labels) Frog Dissection (braille, labels) Frog Dissection (braille, no labels) Frog Dissection (print, grayscale, labels) Frog Dissection (print, grayscale, no labels) Frog Dissection (print, texture, labels) Frog Dissection (print, texture, no labels)

In each of the groups are related files (a JPEG image, Text description, PDF, AI, maybe production notes, etc.) The idea here is one would see initially only the main groups where if they are interested in say the Braille version with labels they would click on that main group exposing all the sub components of that grouping Something like the following:

+ [group image] Frog Dissection (print, color, labels) + [group image] Frog Dissection (print, color, no labels) - [group image] Frog Dissection (braille, labels) (Job would look the same as we have now except maybe indented or within some boarder?) `Frog Dissection (braille, labels) (JPG) [image] [download] [preview] black and white image with braille labels (JPG) ` Type:Image Format: Image - JPG, Accommodations: Visual, Languages: English ...

` Frog Dissection (braille, labels) (PDF) [pdf icon] [download] [preview] black and white image with braille labels (PDF) ` Type:2.5D Tactile Graphic, Format: Other - PDF Accommodations: Visual, Languages: English ...

` Frog Dissection (braille, labels) (AI) [2.5D icon] [download] black and white image with braille labels (Adobe Illustrator) ` Type:2.5D Tactile Graphic, Format: Tactile - AI, Accommodations: Visual, Languages: English ...

    etc...

+ [group image] Frog Dissection (braille, no labels) + [group image] Frog Dissection (print, grayscale, labels) + [group image] Frog Dissection (print, grayscale, no labels) + [group image] Frog Dissection (print, texture, labels) + [group image] Frog Dissection (print, texture, no labels)

If this resource had some lone file not associated with a group, say a video then we would add that either at the end or at the beginning by itself just like it would do currently. What ever is easier, but I wouldn't want it showing up between groups.

attached is an updated excel spreadsheet for the above example which you can use to update the tsv2JSON.py to create the appropriate JSON file for importing. I added 2 made up example

FrogDissection.xlsx resource files at the end not associated with any groups so you can see all possibilities.

sinabahram commented 3 years ago

@jkva please verify the above markup pattern with me before proceeding. I'm imagining headings preceding lists that are aria-labelledby the heading's ID.

jkva commented 3 years ago

@clapierre thanks for the extensive write up 👍 a few questions:

@sinabahram your proposed markup sounds good - I would suggest "headings containing buttons that expand/collapse a list, which is aria-labelled by the heading's ID".

clapierre commented 3 years ago

Hi @jkva,

Yes I would think that the group image could be one of the "image types" files in group, but we would still need the alt-text description right? Now I guess instead of pointing to a URL for the Group image it could point to a unique ID of the image resource file in the group. or were you thinking that the first image file type in the group would always be the "group" image. That could work too I suppose. What ever is easiest.

I wouldn't have thought a file could exist in multiple groups, but if its a "production Notes" that explains how to print out the say tactile it could be duplicated in each group instead of creating a special "color version", "B&W version", etc. so lets not exclude that if its not a lot of extra work. If it is I don't mind creating the same file multiple times with different names to put in each group.

Knowing the size of the group may be useful, would also help I guess with AT knowing how many items they will be encountering when the open this group up. If it has the same UI with the # of items in the group with the circle around it that could work.

Thanks

jkva commented 3 years ago

The group image can be marked as decorative, or use the alt for the image that it uses as group image. So that doesn't seem a problem.

clapierre commented 3 years ago

For the Group image, do you think that would need alt-text or would it be decorative since it has the name of that image along side it. Thoughts @sinabahram

clapierre commented 3 years ago

Thanks @jkva we were thinking the same thing LOL.

jkva commented 3 years ago

@clapierre I suspect the expand/collapse control sitting to the left of the group image is going to look a bit strange, assuming it's going to be a lot smaller than the rendering of the group image. Could you provide a visual mock of what you are expecting?

clapierre commented 3 years ago

Hi @jkva Here is a test page with some options. I like option # 3, but it may not be as accessible as option # 2. Option # 1 uses a table but that is just so I can show you what I was thinking on layout obviously it wouldn't be coded as such.

It would be great if we can get the same type of behavior and look as option # 3 which is accessible.

ImageshareGroup.zip

sinabahram commented 3 years ago

I want to be careful about collapse/expand on the groups. Do we actually need them to be collapsible at all?

clapierre commented 3 years ago

It would be helpful if it could collapse, why is there something wrong with the html summary/details?

sinabahram commented 3 years ago

@clapierre it is just a little bit of an antipattern because it prevents folks from doing a find on the page, as it will all be collapsed. I think making it so that they can be collapsed may be a great idea, but should they start expanded, I suppose is where my head is at.

It just adds several keyboard or mouse operations to get to anything now on that page.

clapierre commented 3 years ago

Can we leave this design decision until after the new year, I would like to see what the team thinks if we should be able to expand/collapse etc. or no option to do that. But everyone now is on holidays. I see your point and maybe expanded by default but button to expand/collapse all near the top of the page where it defaults like you say to expanded.

sinabahram commented 3 years ago

@clapierre and I spoke about this on our call today.

@jkva please tell us your thoughts about the below strategy.

  1. We make a CPT called something like ResourceFileGroup
  2. This CPT is just a grouping container for ResourceFiles
  3. There is associated UI on the backend just like with Resources and Collections where Charles and others can add ResourceFiles to the ResourceFileGroup. This should of course be filtered by only those ResourceFiles that are part of a given Resource e.g. if Charles goes to the frog example with 49 ResourceFiles, and he only sees those 49 ResourceFiles when he selects which ones to add to a given ResourceFileGroup for Frog. Additionally, if a ResourceFile laready belongs to a ResourceFileGroup, it isn't able to be added to another ResourceFileGroup e.g. ResourceFileGroup membership is mutually exclusive.
  4. ResourceFileGroups must have a title. This is exactly what will appear on screen as the heading.
  5. ResourceFileGroups will be distinctly sectioned. All this means is that they will be collapseable (starting exapnded by default) and color and semantic regions will to be used to separate them from each other on the page.
  6. Because We will have multiple groupings on the page, and because on any nontrivial Resource, this will result in scrolling, let's generate a very simple list of same-page links up top to jump to each ResourceFileGroup.
  7. For any ResourceFiles that are not in a ResourceFileGroup, they should appear in the default ResourceFileGroup whose name is TBD (we will come up with something meaningful). This is also useful for those Resources where thre's only 2 or 3 ResourceFiles and so grouping is not necessary (so they will just have their default ResourceFileGroup).b
jkva commented 3 years ago

If I understand this correctly this ticket is no longer concerning a grouping "by type", but more generally the creation of arbitrary resource file groups which can then be linked to a resource.

All the above seems doable, while introducing some added complexity: currently a resource knows about a set of resource files that are its "children"; these children will need to become a set of "resource file groups" which themselves contain references to their resource file children. This impacts the way search relevance is indexed and some other calculations, such as the total amount of files contained by a resource, and the different file types that are contained by a resource.

This also seems to imply (point 7) that when importing a new resource, it needs to implicitly create and associate with itself a "default resource file group" which will contain any resource file definitions in the JSON. I would suggest this group to have a default flag. This would ensure that when rendering a resource which has only a default group as its groups, the contained files are rendered as a flat list the way they are currently rendered.

Can a ResourceFileGroup belong to more than one Resource?

clapierre commented 3 years ago

@jkva No a ResourceFileGroup can only belong to a single Resource. I believe the same is true for a ResourceFile can only belong to a single Resource as well, which aligns with Sina's point 3 above where only Resource Files for a particular Resource are available to choose from to make up a Resource File Group.

jkva commented 3 years ago

@clapierre Does the design for this still need discussion?

I'm working on the last parts for integrating the resource groups - one thing I'll add for that is a tool on the settings page that allows you to "verify" that any existing resource contains a default resource group, this way any pre-existing resource can be migrated to support this changed data model.

clapierre commented 3 years ago

I think we are set unless you run into any questions while in development. Would we be able to just add the default resource group to all existing resources, then for those resources that we want to create separate groups I would just go into those and move some resource files from the default main group into separate new resource file groups that I would create on the fly.

Also if we move all resource files from the default group that group would go away until say we remove a resource from an existing file group (not sure if I would need to do that unless we discover a file was added to a group by mistake) or if we add a new resource file to this resource.

jkva commented 3 years ago

@clapierre I've pushed changes that implement file groups functionality. Some clarification:

File groups type

This is now available as custom post type, and looks like this:

A screenshot of the wp-admin interface showing a table of file groups

The column "Resource" shows the resource the group belongs to, and the "Default for parent resource" tells you whether this group is the "default group" which is rendered without a heading and/or expand/collapse functionality.

File groups are shown for the resource overview

A screenshot of the wp-admin interface showing a table of resources

The "groups" column in this table tells you the amount of groups the resource relates to. The default group is included in this amount.

Resource files show their containing group

A screenshot of the wp-admin interface showing a table of resource files

The "group" column shows the group the file is a member of.

Toggling groups

https://user-images.githubusercontent.com/217264/111213771-6d19d280-85d1-11eb-8fb1-3155b3821c12.mp4

The video above shows a resource with a default group (it has no heading and a few files) and a group called "Fusion" below it, which can be toggled. The styling is pretty basic and I expect this to change, but for now it illustrates the functionality.

Some caveats

After about 2300 resources it for some reason slows down to a crawl. @sinabahram and I know the struggles of batch processing in WPEngine, sadly. For now I have the script detect it and decrease the batch size. It will other than that keep trying to finish the job, just slowly.

A "tools" section on the settings page showing a button named "Ensure Groups"


Let me know if anything is unclear and whether the functionality is as you expect it :)

clapierre commented 3 years ago

@jkva, I am playing around with this and found a bug which I will file separately regarding searching of File Groups from the Resource itself.

I am working with the Frog Dissection and creating multiple File groups for the various related files. My main issue is that it's not easy to create a file Group with a set of files from the Default Group.

What you have right now is the ability to create a new group from ALL files (which is fine, but not really that helpful)

So what I had to do was the following:

  1. Create a new Group called "Frog Dissection Braille Labels"
  2. Search all the files in the database and add only those I want in this group
  3. Find the Frog Dissection Default Group, and delete those same files from that group.
  4. Go into the Frog Dissection Resource and add the new File Group (which was difficult due to the issue I will be filing.)

Thats a lot of steps and if the files are not named very well, will take a lot of work to be able to create this new File Group.

I would like to be able to

  1. Find the Frog Dissection Default Group (which we have now and is easy to do)
  2. Select those files from the default Group that I want to create a sub group from (i.e. all related print, color, labels .jpg, .PNG, alt etc.) which are part of the default group and in one step remove them from the default group and place them into a new Group which I would label and ideally add this new group to the resource itself.

We can leave the way you have this as well incase we want to create a File Group not from a default group, but being able to create subsets from a default group will be the default action and right now it is very cumbersome.

jkva commented 3 years ago

@clapierre that's useful feedback, thanks.

Say you would create a subgroup out of all the "Frog Dissection" files in the default group. I'm assuming you would expect to move the files to the subgroup, as they would then otherwise be members of two groups, right?

So on the edit page of a file group you'd expect a button "Create subgroup", you could then provide a title for the group and mark the files that you would want to move there by way of checkboxes. Once you confirm, the group is created and the files are moved. You would then return to the file group edit page and repeat the process as often as you wanted, granted there'd be still files in the group.

cc @sinabahram

clapierre commented 3 years ago

@jkva

Say you would create a subgroup out of all the "Frog Dissection" files in the default group. I'm assuming you would expect to move the files to the subgroup, as they would then otherwise be members of two groups, right?

Yes when you select the files from the default group and create a new subgroup then we would move them from the default group and into this new subgroup so they would only appear in one group. There is no need to have the same file in multiple sub-groups. I could see the same file maybe a doc file explaining how to print the tactile that one may want to appear in multiple sub-groups potentially, but for now we can just have that same file duplicated with a slightly different name if it makes this easier to implement and more straight forward.

So on the edit page of a file group you'd expect a button "Create subgroup", you could then provide a title for the group and mark the files that you would want to move there by way of checkboxes. Once you confirm, the group is created and the files are moved. You would then return to the file group edit page and repeat the process as often as you wanted, granted there'd be still files in the group.

Yes on all accounts. So the default set of files should keep getting smaller until ultimately there are none left and all the files are part of a sub group. Now thats what I think would be the behavior there could be one or two files which didn't belong to a specific sub-group (maybe even that applies to all groups like how to print the tactile images (size of paper etc.) which would just be in the default group and this might be the way to handle that case as well in which case that final file would just be left in the default group and shown first in the UI.

jkva commented 3 years ago

@clapierre @sinabahram I've been looking into this functionality and it pretty much requires me to implement a complex custom "advanced custom field" field type - effectively a modified "relationship" grouping field which can have subgroups and have files moved around. A feature like this would likely take considerable time to implement.

Alternatively (or in the mean time) what I could do is: you create a new group, add the files to it that you want, and the unique membership of the files to the group is subsequently enforced by the application removing them from any other group(s) they belong to. The only cumbersome part is that you would still need to add the new file group to the resource's list of groups.

I'm trying to consider a pragmatic solution - let me know what you think.

clapierre commented 3 years ago

This could work but could we limit the files they can choose from to a resource set of default files? Make it easier to find them initially.

So when you create a new file group it asks for the resource this will belong to and then only it's files can be selected. Might make it easier to associate this group with the resource afterwards as well?

jkva commented 3 years ago

@clapierre I've inverted the logic so that:

1) resource file groups have a "parent resource" relationship 2) resource file groups have a "is default for parent" setting 3) files being assigned to a file group are automatically removed from any other file groups they belong to 4) a resource file group / resource parent relationship is unique, you can not set a file group as default if the parent resource already has a default file group (see screenshot)

A screenshot of the UI showing an error message when trying to set a file group as default for a resource which already has a default file group

I've had to migrate some data around, there's a new tool for that on the settings page. This has already run and does not need to be run again.

Let me know if you have questions!

clapierre commented 3 years ago

This is looking great, I created a new Resource File Group under the Frog Dissection and it was a lot easier now, with removing the resource files from the default group and associating this new resource group with the resource so that is awesome!

The only other slight improvement would be if we could show all the current files in the default group once you select the parent resource.

So right now when you don't select a parent resource one has the ability to search "all" the resource files which is great and you can then click on them to add them to the right panel of resource files for this new group. So let's keep that.

However, in addition, would it be possible once you then select the "Parent Resource" that the list of files to choose from could get populated by those files in the default group of the selected parent resource, instead of still showing all resource files in the system which makes it hard to just pick from the subset of default resource files for that resource.

sinabahram commented 3 years ago

@jkva can we just show only the files related to that resource, forcing the resource to be picked first? Could solve all the issues, and be more performant.

clapierre commented 3 years ago

Made a separate issue for the filtering bug please see #274

jkva commented 3 years ago

I've addressed #274 (at least partially), @clapierre does this ticket need to remain open?

jkva commented 3 years ago

Closing this as ordering is covered by ticket #268.