search

silverstripe / silverstripe-framework

Silverstripe Framework, the MVC framework that powers Silverstripe CMS
https://www.silverstripe.org
BSD 3-Clause "New" or "Revised" License
721 stars 821 forks source link

Global archive view ("Trashcan") #6202

Closed sminnee closed 6 years ago

sminnee commented 8 years ago

As a user, I want to be able to intuitive retrieve archived records.

Acceptance Criteria

Notes

Related

Subtasks

PRs:

Related Outstanding Issue:

clarkepaul commented 8 years ago

A few years ago there were some suggestions made to help people find deleted/archived pages which could be worth testing against the "trash" idea. @sminnee It might be a good idea to adjust the acceptance criteria to just provide the desired outcome rather than the approach. I intend to do some quick visual testing to get a preferred option—I doubt there is an option which will 100% satisfy everyone.

pasted_image_20_10_16__11_00_am

pasted_image_20_10_16__11_19_am

sminnee commented 8 years ago

I'd be more comfortable with altering the acceptance criteria after we get some different design feedback, otherwise it's too vague to build from. Feel free to post more designs and/or counterpoints here, though.

Firesphere commented 8 years ago

What about emptying the trash?

sminnee commented 8 years ago

That's where the metaphor breaks down, I guess - we never empty the trash ;-)

dhensby commented 7 years ago

What about emptying the trash?

you can write that bit, @Firesphere ;)

sminnee commented 7 years ago

No, emptything the trash is an anti-feature.

The best approach would be to be able to specify a retention policy for a site, where you for example keep:

And the system automatically deletes versions based on those.

jonom commented 7 years ago

I like the term 'Archive' better than 'Trash' for describing what is actually happening. As already pointed out, I think users would expect to be able to empty the trash. They also might not expect that a trashed item is recoverable, or expect that it will only be recoverable for a short time.

I like the other suggestions though, and think they'd work just as well without using Trash as the label. E.g.

A new view on the page list, "Trash" is available --> A new view on the page list, "Archived" is available

Bit of a segway but I also like the retention policy idea! There is a module or two out there for cleaning out old versions to reduce database size but they're not super stable. Does seem like a good fit for a module though as opposed to a core feature.

phptek commented 7 years ago

The idea of an archive and whatever preceeded it (I forget already) papers over the real thing that I as a user really want: "To delete stuff I don't want". If I "archive" it, some additional and unwarranted cognitive activity is invoked, and I wonder "where is it, really?". "Is it gone?", "Is it sort of gone?", "Is it in draft, or live?", "Has it gone on its OE?".

"Trash" says it all. It's been around forever, it's familiar. It means something and I neednt think too hard about it, if at all.

sminnee commented 7 years ago

@clarkepaul can we please organise some user testing around different ideas for this before we start on dev?

clarkepaul commented 7 years ago

@sminnee sure, is there a timeframe I can work to?

The main issue for the users I previously tested with didn't know how to retrieve deleted/archived pages, so I think that really needs to part of the issue and any proposed solutions need to include the testing.

chillu commented 7 years ago

@clarkepaul This is currently in the beta2 milestone, which is expected to be eight weeks duration. Meaning we need user testing in the next two weeks, or the feature drops out of the release (we're not doing major new UI features from early Sept, and start stabilisation and bugfixes only). How realistic is that, given all the other testing going on? We could make this a 4.1 feature?

clarkepaul commented 7 years ago

I'm all good with moving this, as you have already.

patricknelson commented 6 years ago

I agree, I personally don't see much utility in being able to "empty the trash" since that may be a bit too destructive (especially if any single particular user does it inappropriately), however I think it's perfectly fine for a user to still be able to select an individual page (or maybe pages) to remove. Naturally, page permissions on that particular SiteTree descendant should still apply here.

I know it may seem like feature creep, but in situations like these, it's nice to still have some kind of trail or audit log so we can trace what happened (something only accessible to administrative users). It could initially just start as a log of deleted pages (now that the page is completely removed, it's just an entry indicating what was removed and that's it) that could eventually make its way to a full blown auditing system later own down the yellow brick road (if we were ever so inclined).

chillu commented 6 years ago

@clarkepaul @newleeland We're adding archiving toggles in ModelAdmin/GridField as well as AssetAdmin, and not using the "trash" metaphor there.

How does this affect the design approach for this card? Technically, by solving the UI issue for GridField, we could implement it in the pages list view as well. That would conflict with the existing mode switches through the page search though ("page status" dropdown). It sounds like this card is generally still valid, we need a mode switch. But the pattern being established in those other cards seems to move this out of the search context, and into a toggle alongside GridField (e.g. for ModelAdmin).

For reference, here's the current page implementation:

image

chillu commented 6 years ago

I've discussed this with @clarkepaul and @newleeland, and we're trying to broaden this to all records (pages, blocks, etc), in a unified section. Has there been any user testing done on the "archive" vs. "trash" wording? There's more discussion on https://github.com/silverstripe/silverstripe-cms/issues/1575.

I'm wondering if a toplevel section would be discoverable enough by authors. Can you have a think of how users would make the mental connection between archiving a record in one UI, and viewing it in a different UI? I've added an AC to that effect:

I have clear guidance on how to view archived records wherever this can be triggered in the UI

Previous ACs:

newleeland commented 6 years ago

Potential design Phase 1 https://invis.io/NP79NDU4J#/278379147_Archive_-_Global_Tab_-_Pages_-Default- There are 4 Main areas - Pages, Blocks, Files, Other (All other models)

Potential design Phase 2: Bulk actions https://invis.io/NP79NDU4J#/278379342_Archive_-_Global_Tab_-_Bulk_Actions

What are your thoughts @clarkepaul @chillu

chillu commented 6 years ago

The detail view seems to mix "inline editing" for the page (single table row at the top) with our standard detail view (breadcrumbs at the top, form in the middle, south toolbar). I haven't seen this pattern before, and don't think it's something we should introduce due to complexity. There's a need to show some readonly data ("date archived", "archived by") on detail views of record versions. But that would be more easily done through more readonly form fields (or maybe a separate top section), not a table with a single row.

This would also work better with the MVP of showing those version views in their original context (e.g. the "pages" section). You haven't marked any toplevel sections as active in your designs, can you confirm that it's still the direction for "Phase 1". I know you'd prefer to keep it in the "Archive" toplevel section, but we'll only do that if it's a small amount of dev work.

Bulk actions as "Phase 2" seem fine, although I don't think we need to create specific designs for each bulk use case there - that should just be a generic pattern that can be applied to GridField regardless of the action taken (publish, delete, restore, etc)

/cc @clarkepaul

chillu commented 6 years ago

@newleeland @clarkepaul I think we still need some UX answers on how CMS users find their way to this view in the first place.

Example use cases with unclear paths:

I've just deleted a content block, and changed my mind while I'm still on the same screen (page view that formerly contained the content block)

I've realised that I've accidentally deleted a page yesterday. It no longer shows in the page tree, I can't find it through search, and there's no indication on the parent page.

A colleague of mine deleted a file I still needed. They didn't remember where it was stored. I went to the files section and can't find it anymore, even when searching for a few keywords.

newleeland commented 6 years ago

The detail view seems to mix "inline editing" for the page (single table row at the top) with our standard detail view (breadcrumbs at the top, form in the middle, south toolbar). I haven't seen this pattern before, and don't think it's something we should introduce due to complexity.

Yes. this is a new pattern that was introduced with our new history viewer we are making, along with the new content block work. (see prototype https://github.com/silverstripe/silverstripe-versioned/issues/37) Would this make it easier to develop?

This would also work better with the MVP of showing those version views in their original context (e.g. the "pages" section).

We originally were going to keep how we view archived pages the same, however, we would still need to create a new template from Blocks/Files/Other models anyway. And as initial tests suggest restoring pages were a little confusing, we thought we could just use the template used by block/files/other models instead. Would this work?

Updated Phase 1 prototype. I've marked all the top level sections active where required ~https://invis.io/NP79NDU4J#/278379147_Archive_-_Global_Tab_-_Pages_-Default-~

Potential design Phase 2: Bulk actions https://projects.invisionapp.com/share/NP79NDU4J#/278379342_Archive_-_Global_Tab_-_Bulk_Actions

Viewing the trash Placing the trash button on the bottom of the side navigation on left, means that:

This examples, would be addressed more by Alert noticification which would offer options to undo the users previous acitons

I've just deleted a content block, and changed my mind while I'm still on the same screen (page view that formerly contained the content block)

This is where a global trash (left navigation) would work very. There is one place for the user to search.

A colleague of mine deleted a file I still needed. They didn't remember where it was stored. I went to the files section and can't find it anymore, even when searching for a few keywords.

I've realised that I've accidentally deleted a page yesterday. It no longer shows in the page tree, I can't find it through search, and there's no indication on the parent page.

chillu commented 6 years ago

Yes. this is a new pattern that was introduced with our new history viewer we are making, along with the new content block work. (see prototype silverstripe/silverstripe-versioned#37) Would this make it easier to develop?

Right. We're planning to redevelop the history viewer as a React UI component, so it's a bit easier to accomodate these things. In general, mixing readonly views (single table row in detail view) with editable data makes the UI harder to develop, since you need to keep that state in sync or cause author confusion. It also means the view becomes specific to that grid field with those particular columns. In an ideal world, we have one edit view for a record that can be reused regardless of the context that triggered it. A record might be in multiple different grid fields, each with their own columns. Architecturally, we want to separate master and detail components from each other both on the view and the API level, and this design patterns makes this harder.

The single row also shows sort controls on each column, which don't really make sense in the single-row detail view. What would happen if you click them? Navigate back to the list view and update the sort? That's the first time I've ever seen a pattern like this in a UI.

A full blown React grid field would allow for easier inline editing ("expand the row"). That would not hide the other rows though, so different from the design. And it would keep the breadcrumbs of the original view, not change it to the detail view. I think this needs more thought if its a pattern beyond the history viewer, where it makes more sense given the "compare" relationship between two rows.

Were you meaning to comment on the last two scenarios? You quoted them without new content.

newleeland commented 6 years ago

With meeting between @chillu, @clarkepaul and myself, we decided to remove the single row table in favour of having the data displayed underneath the content preview, with read-only form fields. Updated designs https://invis.io/FTG16IYV5KB#/280476971_Archive_-_Global_Tab_-_Pages_-Default-

chillu commented 6 years ago

I've left a comment on the term archive is confusing about the inconsistent "Trash" naming in the designs.

  • I can understand why records can't be permanently deleted (no "empty the trash" feature)

@newleeland How do you think we should communicate this? If we call it "trash", then users will assume there's a way to clear the trash (there isn't, and shouldn't be). If we call it "archive", it's a bit clearer, but users will still be confused. I suggest a sentence at the bottom of the new section list views stating something like "Records here are archived, and no longer viewable by unauthenticated users. Permanent deletion is disabled in order to preserve an audit record."

  • I have clear guidance on how to view archived records wherever this can be triggered in the UI

It sounds like @newleeland is fine with not mentioning this explicitly in the context of where records gets archived, and trust that users will find their way to the new "archive" section independently. I've removed the AC.

clarkepaul commented 6 years ago

+1 on you comments @chillu. I think we can communicate where items have been archived by having a badge on the "Archive" when items are added to it.

@newleeland I would like to see more of a pattern around when/if the "Undo" link in the alert would be used.

chillu commented 6 years ago

I think we can communicate where items have been archived by having a badge on the "Archive" when items are added to it.

Would that be session based? Or disappear once you move away from the page? It might be mistaken for something you have to look at as an author (like a direct message from another author, or an important system notification), which isn't the case here. But happy to hear some user testing feedback on that approach.

clarkepaul commented 6 years ago

@newleeland I want to see the full process from archiving an item to retrieving it as there would be notifications and perhaps "Undo" actions? which all play a part of making this global archive work. @chillu I was thinking it would be just a temporary indicator (5 sec type thing) but this is a nice to have at this stage—would also depend on what other notifications are present when an item is archived.

chillu commented 6 years ago

Went through this on backlog planning, just to summarise for @clarkepaul

newleeland commented 6 years ago

Previewing Archive https://invis.io/FTG16IYV5KB#/280476971_Archive_-_Global_Tab_-_Pages_-Default-

Restoring Trash (Direction 2) (text needs refinement) @clarkepaul @chillu https://invis.io/FTG16IYV5KB#/282833496_Archive_-_Global_Tab_-_Restoring_Pages_Start_C2

Relevant AC's which differ from the top: - If I mistakenly restored an item, I can quickly undo it.

chillu commented 6 years ago

I am notified when a restore is successful

Added that to the ACs

If I mistakenly restored an item, I can quickly undo it.

Marked as out of scope, see my previous comment.

@newleeland Please read my comment regarding info messages again. I've asked a few times now that we clarify to the user why items can't be permanently deleted. I don't see it in the designs.

newleeland commented 6 years ago

Permanently delete design The use case, as mentioned by @chillu, is for the users who are looking to permanently remove items from the CMS. Notifying them that the CMS archives is not used for this functionality. Directing them to a developer to remove such item. There are 3 ways to approach this edge case:

lukereative commented 6 years ago

Proposed approach for including miscellaneous dataobject classes as extra tabs

Create a TabDropdown component that extends TabSet so would leverage the current ability to nest Tabsets.

However this TabDropdown class would create a tab/button on the frontend which would use entwine to connect mount a react dropdown that displays the respective list of tabs. We could do it in a similar way to GridField_ActionMenu which loads a schema into a data attribute which is interpreted by React when mounted.

The TabDropdown react component could also be used complete this story which has been floating around for a long time https://app.zenhub.com/workspace/o/silverstripe/silverstripe-framework/issues/1943 – the only functionality that would need to be added would be determining which tabs are overflowing

chillu commented 6 years ago

Currently those tabsets are created through server-generated markup. They're also styled differently from getCMSFields() tabs (don't have nav-item, not inheriting Bootstrap styles?).

Bootstrap documents using tabs with dropdowns, and rely on aria-expanded for this case - which seems to preserve accessibility.

We're kind of going away from the idea of "everything is a form" in our UI, that was more of a means to an end in 3.x. So I'm not sure that toplevel tabs spanning multiple actual forms (each containing a single GridField) should be represented as a TabSet in a form (they're not at the moment). I think we're at a point where "form schema" is containing bits of UI that aren't necessarily in a form. Their data might take the same shape, but it's not a form schema as such (e.g. there's no action attribute).

For this particular ModelAdmin tabset, we could create a GraphQL introspection endpoint to list all versioned classes. For other ModelAdmins, we need a custom list based on the specific ModelAdmin controller settings.

Alternatively, we could make this whole UI a single form with one form schema and different GridFields on an actual PHP TabSet. That'd be quite a refactor in terms of ModelAdmin API surface. And would need to be applied consistently in other areas, e.g. on "Content" vs. "Settings" tabs in the "Pages" section. Which comes at a relatively large performance cost, calling both getCMSFields() and getSettingsFields() - so would require client-side caching of those schemas. Which is difficult to do without immutability in those methods (no conditionals).

chillu commented 6 years ago

I think for now, just skip the whole "tabs as structured data" issue, and extend our <Tabs> component in two ways:

Then just take the server-rendered tabs, parse the structured data out of HTML, and re-initialise and re-render them as a React component. My assumption is that there's minimal visual disruption to users in doing this.

chillu commented 6 years ago

Luke, Aaron and myself had a chat, and we don't want to support a "more tabs" data structure in addition to the existing nested TabSet approach used elsewhere (e.g. in "Pages"). This design was a workaround for keeping things as close as possible to the current ModelAdmin implementation, which turned out to actually make it harder. I've talked to @newleeland about reverting to his original design with a single dropdown UI component instead of tabs. This dropdown would then trigger a URL change and show the archive view for the selected item. He'll put updated designs on here shortly.

Also, we should come up with a way to get "all versioned objects" as structured data, rather than plonking them into a pseudo-form in ModelAdmin. Ideally through GraphQL. But looking at ModelAdmin->getEditForm(), it's going to be way faster to add a server-rendered dropdown with a bit of Entwine customisation for the URL routing. Let's keep larger refactorings to when we address the whole of ModelAdmin. But ensure that the dropdown does the URL change in an accessible fashion.

newleeland commented 6 years ago

Potential design alterations for "Other" tab: https://invis.io/FTG16IYV5KB#/303857930_Archive_-_Global_Tab_-_Others_-Alteration_2-

maxime-rainville commented 6 years ago

All done.