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

Global archive view ("Trashcan") #6202

Closed
sminnee opened this Issue Oct 19, 2016 · 35 comments

Comments

Projects
None yet
@sminnee
Member

sminnee commented Oct 19, 2016

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

Acceptance Criteria

  • I can see which records have been archived (deleted from both draft & published) in a central place
  • I can choose the model to view archived records for
  • All versioned models are available to choose from (works with dozens of models)
  • I can page through long lists
  • I have enough context to tell from the list where a record belongs to (breadcrumbs for pages and files, container page for a block) - unless the record is orphaned
  • By default, items are shown sorted by the date they were deleted, most recent first
  • I can see who archived a record, on overview and detail views
  • I can see when a record was archived, on overview and detail views
  • I can restore a record without clicking through to the edit view (some records won't have an edit view configured)
  • I can understand why records can't be permanently deleted (no "empty the trash" feature)
  • I can click through to a readonly "edit view" (e.g. in the "pages" section) where the record can be viewed and restored. Requires correct setup by devs (DataObject->EditLink())
  • If a record needs to be restored in a hierarchy that no longer exists, it will restore to the top level (mainly pages and files)
  • If a record clashes on its primary identifier (e.g. filename, email), the process will auto-rename to avoid the clash and the user gets notified
  • After restoring a record, it's clear to the author where it went (hierarchy location)
  • I am notified when a restore is successful
  • Optional: I can view the readonly "edit view" in context of the new archive section, and don't get redirected to a different section for it

Notes

  • Designs at https://invis.io/FTG16IYV5KB#/299554599_Archive_-_Global_Tab_-_Menu
  • Excludes searching (separate card)
  • Excludes solving the "archive" vs. "delete" vs. "trash" discussion (see silverstripe/silverstripe-cms#1575) - it's an easy technical fix if we decide to switching wording
  • Excludes bulk restore (separate card)
  • Should reuse as much of ModelAdmin's capabilities as possible to simplify implementation
  • Shouldn't show a combined list of multiple record types since that's too performance intensive to query
  • Centralising this view is an attempt to limit the complexity of different UI implementations
  • This is relatively high priority because we don't have a way to list archived records in the blocks or files UI at the moment. For blocks, you can go through the history of the page containing the block and restore from there, a file and any other records managed through GridField only are "lost" at this point.
  • Excludes "Undo" action for after archiving or restoring

Related

Subtasks

  • Create 'Archive' model admin
  • Create 'Pages' section which shows archived pages
  • Create 'Blocks' section which shows archived blocks
  • Create 'Files' section which shows archived files
  • Create 'Others' section
    • Dropdown which shows versioned models
    • Dropdown styling
    • Generic view which shows the selected model
  • Add GridField 'Restore Item' row action
  • Add 'Restore Item' edit form action
  • Design review with @newleeland
  • Tests

PRs:

Related Outstanding Issue:

@sminnee sminnee added this to the CMS 4.0.0-beta1 milestone Oct 19, 2016

@clarkepaul

This comment has been minimized.

Show comment
Hide comment
@clarkepaul

clarkepaul Oct 19, 2016

Member

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

Member

clarkepaul commented Oct 19, 2016

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

This comment has been minimized.

Show comment
Hide comment
@sminnee

sminnee Oct 19, 2016

Member

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.

Member

sminnee commented Oct 19, 2016

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

This comment has been minimized.

Show comment
Hide comment
@Firesphere

Firesphere Oct 20, 2016

Contributor

What about emptying the trash?

Contributor

Firesphere commented Oct 20, 2016

What about emptying the trash?

@sminnee

This comment has been minimized.

Show comment
Hide comment
@sminnee

sminnee Oct 21, 2016

Member

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

Member

sminnee commented Oct 21, 2016

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

@dhensby

This comment has been minimized.

Show comment
Hide comment
@dhensby

dhensby Nov 7, 2016

Member

What about emptying the trash?

you can write that bit, @Firesphere ;)

Member

dhensby commented Nov 7, 2016

What about emptying the trash?

you can write that bit, @Firesphere ;)

@sminnee

This comment has been minimized.

Show comment
Hide comment
@sminnee

sminnee Nov 7, 2016

Member

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:

  • every single version for 30 days
  • daily versions for a year
  • weekly versions for 7 years

And the system automatically deletes versions based on those.

Member

sminnee commented Nov 7, 2016

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:

  • every single version for 30 days
  • daily versions for a year
  • weekly versions for 7 years

And the system automatically deletes versions based on those.

@jonom

This comment has been minimized.

Show comment
Hide comment
@jonom

jonom Nov 8, 2016

Contributor

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.

Contributor

jonom commented Nov 8, 2016

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

This comment has been minimized.

Show comment
Hide comment
@phptek

phptek Jan 15, 2017

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.

phptek commented Jan 15, 2017

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 sminnee modified the milestones: CMS 4.0.0-beta2, CMS 4.0.0-beta1 Mar 7, 2017

@sminnee

This comment has been minimized.

Show comment
Hide comment
@sminnee

sminnee Mar 7, 2017

Member

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

Member

sminnee commented Mar 7, 2017

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

@clarkepaul

This comment has been minimized.

Show comment
Hide comment
@clarkepaul

clarkepaul Mar 8, 2017

Member

@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.

Member

clarkepaul commented Mar 8, 2017

@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

This comment has been minimized.

Show comment
Hide comment
@chillu

chillu Jul 5, 2017

Member

@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?

Member

chillu commented Jul 5, 2017

@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?

@chillu chillu removed this from the Recipe 4.0.0-beta2 milestone Jul 5, 2017

@clarkepaul

This comment has been minimized.

Show comment
Hide comment
@clarkepaul

clarkepaul Jul 6, 2017

Member

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

Member

clarkepaul commented Jul 6, 2017

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

@patricknelson

This comment has been minimized.

Show comment
Hide comment
@patricknelson

patricknelson Nov 9, 2017

Contributor

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).

Contributor

patricknelson commented Nov 9, 2017

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

This comment has been minimized.

Show comment
Hide comment
@chillu

chillu Feb 6, 2018

Member

@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

Member

chillu commented Feb 6, 2018

@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

This comment has been minimized.

Show comment
Hide comment
@chillu

chillu Feb 8, 2018

Member

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 silverstripe/silverstripe-cms#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:

  • The "Archive" action is renamed to "Move to trash"
  • A new view on the page list, "Trash" is available
  • The Trash shows all the pages that have been deleted from both draft & published
  • By default, trashed items are shown sorted by the date they were deleted, most recent first
  • A restore option is available on pages, either one by one or in bulk mode via checkboxes
Member

chillu commented Feb 8, 2018

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 silverstripe/silverstripe-cms#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:

  • The "Archive" action is renamed to "Move to trash"
  • A new view on the page list, "Trash" is available
  • The Trash shows all the pages that have been deleted from both draft & published
  • By default, trashed items are shown sorted by the date they were deleted, most recent first
  • A restore option is available on pages, either one by one or in bulk mode via checkboxes
@clarkepaul

This comment has been minimized.

Show comment
Hide comment
@clarkepaul

clarkepaul Feb 27, 2018

Member

+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.

Member

clarkepaul commented Feb 27, 2018

+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

This comment has been minimized.

Show comment
Hide comment
@chillu

chillu Feb 27, 2018

Member

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.

Member

chillu commented Feb 27, 2018

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

This comment has been minimized.

Show comment
Hide comment
@clarkepaul

clarkepaul Feb 28, 2018

Member

@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.

Member

clarkepaul commented Feb 28, 2018

@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

This comment has been minimized.

Show comment
Hide comment
@chillu

chillu Mar 4, 2018

Member

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

  • Dropdown for "other" is hard, because GridFields don't deal well with this. We need to investigate how much work it is to implement dropdowns on long tabs, an ancient ticket from 2013. James has already done designs for this, moved the card on top of backlog to estimate
  • We haven't included "undo" actions in the ACs, I think that's better tackled on a broader scale (not just for archiving). @newleeland Do you want to have a think on how that fits in the bigger picture, and what other situations and UI contexts it would be useful in?
  • I've split out the "temporary indicator" to another card: silverstripe/silverstripe-admin#452
Member

chillu commented Mar 4, 2018

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

  • Dropdown for "other" is hard, because GridFields don't deal well with this. We need to investigate how much work it is to implement dropdowns on long tabs, an ancient ticket from 2013. James has already done designs for this, moved the card on top of backlog to estimate
  • We haven't included "undo" actions in the ACs, I think that's better tackled on a broader scale (not just for archiving). @newleeland Do you want to have a think on how that fits in the bigger picture, and what other situations and UI contexts it would be useful in?
  • I've split out the "temporary indicator" to another card: silverstripe/silverstripe-admin#452
@newleeland

This comment has been minimized.

Show comment
Hide comment
@newleeland

newleeland Mar 5, 2018

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.

newleeland commented Mar 5, 2018

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

This comment has been minimized.

Show comment
Hide comment
@chillu

chillu Mar 8, 2018

Member

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.

Member

chillu commented Mar 8, 2018

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

This comment has been minimized.

Show comment
Hide comment
@newleeland

newleeland Mar 19, 2018

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:

newleeland commented Mar 19, 2018

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

This comment has been minimized.

Show comment
Hide comment
@lukereative

lukereative Jun 18, 2018

Contributor

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

Contributor

lukereative commented Jun 18, 2018

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

This comment has been minimized.

Show comment
Hide comment
@chillu

chillu Jun 18, 2018

Member

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).

Member

chillu commented Jun 18, 2018

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

This comment has been minimized.

Show comment
Hide comment
@chillu

chillu Jun 18, 2018

Member

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

  • Instead of toggling, a <Tabs> can get an onToggle prop which uses a callback. In our case, that triggers an Entwine-style PJAX load
  • <Tabs> can pass on a "secondary" boolean prop, which puts specified tabs into a Dropdown. This can later be extended to auto-adjust based on available screen width.

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.

Member

chillu commented Jun 18, 2018

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

  • Instead of toggling, a <Tabs> can get an onToggle prop which uses a callback. In our case, that triggers an Entwine-style PJAX load
  • <Tabs> can pass on a "secondary" boolean prop, which puts specified tabs into a Dropdown. This can later be extended to auto-adjust based on available screen width.

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

This comment has been minimized.

Show comment
Hide comment
@chillu

chillu Jun 18, 2018

Member

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.

Member

chillu commented Jun 18, 2018

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

This comment has been minimized.

Show comment
Hide comment
@newleeland

newleeland commented Jun 19, 2018

@maxime-rainville

This comment has been minimized.

Show comment
Hide comment
@maxime-rainville

maxime-rainville Jul 12, 2018

Contributor

All done.

Contributor

maxime-rainville commented Jul 12, 2018

All done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment