Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Missing "remove connection" from resource #261

Closed
clash99 opened this issue Jun 23, 2016 · 17 comments · Fixed by #644
Closed

Missing "remove connection" from resource #261

clash99 opened this issue Jun 23, 2016 · 17 comments · Fixed by #644

Comments

@clash99
Copy link
Contributor

clash99 commented Jun 23, 2016

Currently resources have archive/unarchive links. We can rename archive to "delete" but unarchive should be removed or hidden to only sys admins. We are missing the "remove connection" link that removes the link to the party/relationship/location/project but keeps it in the resource library.

@wonderchook wonderchook added this to the Version 0.1 milestone Jun 28, 2016
@seav seav added the ui/ux label Jun 29, 2016
@wonderchook wonderchook modified the milestones: Post July Release Sprint, Version 0.1 Jul 8, 2016
@seav seav self-assigned this Jul 21, 2016
@seav
Copy link
Contributor

seav commented Jul 22, 2016

I'm looking at the site, and there is actually a way to disconnect resources from the project, locations, parties, or relationships. It's not intuitive, though. When you select the "Add from library" option in an entity page, the resources whose checkbox is already checked are the ones already connected to the entity. So to disconnect the resource or connect others, just check or uncheck the desired checkboxes, then click "Save".

We should probably review and redesign the UI around connecting and disconnecting resources with their entities. One possible design I can think of is the following:

  • For the table listing the connected resources of each entity, add a button per row labeled "Disconnect" (or "Remove connection"), similar to the "Deactivate"/"Activate" link in the Users page.
  • For the "Add from library" resources table, only list the unconnected resources. This makes the intended action of "adding from the library" more intuitive.

What do you all think?

@clash99
Copy link
Contributor Author

clash99 commented Jul 22, 2016

The wireframes had the ability to remove the connection as part of the detail page. The "permanently delete" should only be viewable by the original owner of the resource or admin, not everyone.

2 4 2 edit resource

@seav
Copy link
Contributor

seav commented Jul 22, 2016

This would be a problem. Right now, a resource can be connected to the project, every location, every party, and every relationship. What does "Remove connection" mean on the resource page? Does this mean remove the resource from everything it is connected to?

@clash99
Copy link
Contributor Author

clash99 commented Jul 22, 2016

Good point and hard to follow on the wireframes. It would be removing the connection from the entity that you opened the resource on so we could change the text to say "Disconnect from this location" or "Disconnect from this project", depending on where you came from. Does that make it clearer?

I also think this would be easier to follow if we implemented the resource detail modal so then you get more of the context without all the options the user probably isn't looking for.

2 4 3 resource detail modal

@oliverroick
Copy link
Member

This is currently not possible. There is only one view for resource details. Not matter where you're coming from (location, party, relationship) you will always end up on that same view without knowing where the user came from.

The only idea to solve this is to create separate views for each location resource detail, party resource detail, relationship resource detail and project resource detail. So for the detail views, we would need to maintain four views and four templates. I'm not convinced that a feature as tiny as this is worth going that way.

@clash99
Copy link
Contributor Author

clash99 commented Jul 25, 2016

@oliverroick So we are going to be able to add the "remove connection" action, correct? Just not customize the message? I just want to make sure I understand.

@wonderchook
Copy link
Contributor

So I'm going to move this off of this sprint and then we can review again for the next release.

@wonderchook wonderchook modified the milestones: Version 1.0, Post July Release Sprint, Sprint 8 Jul 25, 2016
@seav
Copy link
Contributor

seav commented Aug 29, 2016

Here's my proposed enhancements for resolving this issue:

  1. Rename the action of adding a resource to an entity (project, location, etc.) to "attach" and the action of removing a resource from an entity to "detach". I feel that "attach" and "detach" better explains what the actions do than "add" and "remove". "Remove" might be confused with "delete", for instance.
  2. Rename "Archive resource" to "Delete resource" and "Unarchive resource" to "Undelete resource", and only show the latter to superusers. Archived resources are not actually deleted but are rendered invisible to non-superusers, same as with archived orgs and projects.
  3. Archiving a resource detaches it from all entities it is attached to. Unarchiving a resource does not reattach it to the entities it was attached to, but merely makes it visible in the resource library.
  4. For the project resources page, only show archived resources to superusers. This is so that superusers can get to archived resources and possibly unarchive them.
  5. Make attaching resources from library only about attaching resources instead of the current behavior where all resources are shown and you can attach or detach resources as desired. This means that the "Attach from library" table only shows unattached resources. If all resources are already attached, the user is only presented with the upload resource form.
  6. Provide 2 ways to detach resources:
    1. For each table that shows a list of attached resources (such as the "Resources" tab of the location detail page), add another column of "Detach" buttons to detach the corresponding resource. For small tables (both the table_sm.html template and the table.html template that is collapsed in small screens), replace the "Connections" column with this new detach column.
    2. For the resource detail view, add another table below listing the entities to which it is attached to. This table also provides a column of "Detach" buttons as well.

I'd like to get feedback on these proposed enhancements.

@seav
Copy link
Contributor

seav commented Aug 30, 2016

I've finished coding item № 6.i. above and here is how they look like:

As mentioned in № 4, the project resources list page shows all resources in the project, not just the ones attached to the project itself. For superusers, archived resources are also shown as seen below. For non-superusers, archived resources aren't shown. Resources that are attached to projects have a "Detach" button that lets the user detach this resource from the project, but retains the resource in the library.

screen shot 2016-08-30 at 17 55 31

The resources tab of locations show the following table. Each resource has a "Detach" button that lets the user detach this resource from the location.

screen shot 2016-08-30 at 18 10 01

As always, feedback is very much welcome!

@wonderchook
Copy link
Contributor

@seav looks good to me. Thinking maybe we should use a button that says "archived" instead of deleted. What do you think?

@clash99
Copy link
Contributor Author

clash99 commented Aug 30, 2016

@seav - good stuff! just a couple of comments:

  • I think the term "deleted" is probably okay in this instance since unlike other areas in the platform, the typical user can't "unarchive".
  • I think when deleting a resource that has attachments to other entities, we should consider how to inform the user that those connections will be removed as well.
  • We should replace the word "connections" with "attachments" through resource area (Columns of Resources tables for each type of entity is not consistent #497).
  • There isn't really a way that any user would know that a sysadmin could recover a mistakenly-deleted resource file. We should probably include something like a contact link in the user guides.

I would love to review it once developed to review the interaction. Is this in a branch?

@seav
Copy link
Contributor

seav commented Aug 31, 2016

@wonderchook: given the suggestion that the UI should treat archived resources as if they are deleted, then I think it would be best to consistently use "delete"/"deleted"/"undelete" terms in the UI.

@clash99:

  • I can add some additional text to the modal that confirms if the user really wants to delete the resource. It currently says: "Are you sure you want to delete this resource?" We can add something like "Deleting will also detach the resource from everything it is attached to."
  • For Columns of Resources tables for each type of entity is not consistent #497, I'm also resolving that in the code updates. As you can see in the screenshots above, I have already replaced "Connections" with "Attachments".
  • While adding a contact link is a good idea, I think we can also add this to the confirmation modal: "Once deleted, the resource can only be restored by a system administrator. [Reminder of steps on how to contact an administrator, if it's not obvious in the platform.]" What do you think?
  • The current code is currently in a branch: enhancement/Missing "remove connection" from resource #261. I would love it if you can try it out before I submit a PR. 😀

@seav
Copy link
Contributor

seav commented Sep 1, 2016

I've finished coding item № 6.ii. above to display a list of entities a resource is attached to in the resource's detail page, with the option to detach the resource from any of those entities. Here is how it looks like.

screen shot 2016-09-01 at 12 02 13

As always, feedback is very much welcome!

seav added a commit that referenced this issue Sep 1, 2016
- Improve the feature of attaching/detaching resources
- Treat archived resources as if they were deleted
seav added a commit that referenced this issue Sep 1, 2016
- Improve the feature of attaching/detaching resources
- Treat archived resources as if they were deleted
@clash99
Copy link
Contributor Author

clash99 commented Sep 1, 2016

@seav - wow, you've done a lot of work on this one - looks good!

Nothing major but I've made some ui changes in the branch #261-addon. Use at your own discretion :)

For small resources table within a location:

  • Used condensed table due to real estate constraints
  • Minimized some padding and margins to make better use of space
  • Removed # of other attached entities... thought about this one a lot but came to the conclusion that it doesn't add a lot here
  • Removed "attached" column header
  • Sized down minimum button width for the detach buttons in css

Here's a question I'm still thinking about: are we making the detach button a bigger option than it should be? Would it be a more common request to download the resource than to detach it? Should we give the detach action less prominence? What do you think? Maybe we mark this one for testing in the future.

screenshot 2016-09-01 13 17 33

For resources table:

  • Changed "deleted" text to label since we are doing something similar for "archived" projects and orgs to continue that pattern. I know it competes a bit the with the "detach" button but since this is only for sys admins, I think it will work.
  • Separated attached column from action column
  • Added "restore" button directly within table (similar to users table pattern) THIS ISN'T WIRED CORRECTLY

screenshot 2016-09-01 15 34 47

Resource detail page:

  • Changed "Attachments" to "Attached to"

screenshot 2016-09-01 15 44 25

Resource index (used for party, etc.):

  • I didn't make any changes specifically but it feels like we should have an entity filter on this page that would match what is seen on the resource detail page under entity class. That would be the only way I could find a project-level resource. The way it currently is now, they are all mixed together. Perhaps a dropdown choices of All, Project, Location, Relationship, Party. Other thoughts?

@seav
Copy link
Contributor

seav commented Sep 2, 2016

@clash, some replies:

Here's a question I'm still thinking about: are we making the detach button a bigger option than it should be? Would it be a more common request to download the resource than to detach it? Should we give the detach action less prominence? What do you think? Maybe we mark this one for testing in the future.

One possibility is to remove the detach buttons here and only provide the detaching function on the resource detail page.

Another option just to reduce the visual prominence is to replace the buttons with checkboxes and have a "Detach selected" button after the table to complement the Attach button. (We can also combine this with a selectable option to detach or delete (archive) the selected resources.

I didn't make any changes specifically but it feels like we should have an entity filter on this page that would match what is seen on the resource detail page under entity class. That would be the only way I could find a project-level resource. The way it currently is now, they are all mixed together. Perhaps a dropdown choices of All, Project, Location, Relationship, Party. Other thoughts?

Well, we do already have #502 that was filed by you and targeted by Kate for the next Sprint. I think we can leave this out for now and discuss again during the next Sprint. But at least the current table which shows the library, instead of just project-level resources as before, provides a better base on which to build the filtering, don't you think?

As for the other changes, I'm incorporating them as they are all sensible decisions. I am just having a bit of a trouble wiring up the "Restore" button on the project resources page as this requires a polymorphic confirmation modal. I'll try to finish things up so you can further review this.

seav added a commit that referenced this issue Sep 2, 2016
- Improve the feature of attaching/detaching resources
- Treat archived resources as if they were deleted
@clash99
Copy link
Contributor Author

clash99 commented Sep 2, 2016

@seav -

  1. We should leave the detach buttons because I think it makes sense right now. The other option of adding a small download button/link and smaller detach button/link will be good test material for the future.

  2. I agree the new resource table is better and adding a filter in Add filter on resource index page #502 will only increase its worth.

I like having these discussions so I can bounce ui ideas around. These changes really improve the resource area's usability - thanks!

@seav
Copy link
Contributor

seav commented Sep 2, 2016

@clash, I have updated enhancement/#261 with your changes. Please do review the interactions, especially with the Restore button on the project resources page, which I have now wired up.

clash99 pushed a commit that referenced this issue Sep 2, 2016
#261

# Conflicts:
#	cadasta/core/static/css/main.css
#	cadasta/core/static/css/resources.scss
#	cadasta/templates/resources/project_detail.html
#	cadasta/templates/resources/table.html
#	cadasta/templates/resources/table_sm.html
seav added a commit that referenced this issue Sep 2, 2016
- Improve the feature of attaching/detaching resources
- Treat archived resources as if they were deleted
ian-ross pushed a commit that referenced this issue Sep 6, 2016
- Improve the feature of attaching/detaching resources
- Treat archived resources as if they were deleted
seav added a commit that referenced this issue Sep 8, 2016
- Improve the feature of attaching/detaching resources
- Treat archived resources as if they were deleted
seav added a commit that referenced this issue Sep 8, 2016
- Improve the feature of attaching/detaching resources
- Treat archived resources as if they were deleted
oliverroick pushed a commit that referenced this issue Sep 8, 2016
* Resolve #261:
- Improve the feature of attaching/detaching resources
- Treat archived resources as if they were deleted

* Resolve PR #644 feedback; rework permissions functionality; do other tweaks

* Resolve further PR review feedback
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants