History Viewer: Diff on GridFields

[chillu]

Acceptance Criteria

• When viewing a version of a record with versioned GridFields, I can see changes to this relationship between the two versions
• I can see added and deleted items
• I can see changes to sort order of items
• I can see the changes roughly in the same presentation as when editing the relationship (e.g. in a table)
• When viewing changes in a field with more detail, I can click through to the detailed compare view
• Works with 100s of records (needs pagination)

Notes

• Styleguide Gridfield designs
• Diff Relationship designs:
  https://projects.invisionapp.com/share/FTG16IYV5KB#/screens/287812383 which builds upon basic history viewer designs https://projects.invisionapp.com/share/FJEHOLLEN#/screens/263513634_Pages_History_Tab
• Does not apply to unversioned relationships
• Independant of ownership status on relationships
• CCers are building React pagination for version listing
• Assumption: No ORM changes needed, limited GraphQL API changes
• Works regardless where GridField is located (incl. pages section)
• Some records can't be identified by a single value (e.g. Member needs firstname + last name). This is a similar issue to the new archive section. This "identifier value" should be configurable, and work with the same APIs as this new archive section

Related

• History Viewer: Diff on UploadField
• History Viewer: Diff on ListboxField and MultiSelectFIeld
• Useful GridField readonly view

[chillu]

@clarkepaul @newleeland This might still need designs?

[clarkepaul]

Seems the main things which still require design are:
GridField, UploadField, ListboxField, MultiSelectField, and changes to sort order of items.

We can see if there are worthwhile improvements to changes in a field but I was just going to stick with the current pattern (green background for new, red for removed).

[NightJar]

Seems tightly related to the use case described for silverstripe/silverstripe-elemental#92

[sachajudd]

I'll be coming up with a few different design concepts over the next few weeks 👍

[sachajudd]

As a file has its own version history and can be viewed somewhere else, we are trying to figure out the level of clarity we need to surface on the page/block about the changes which have occurred in comparison mode.

WIP variations: (Note these concepts are to get an initial design direction)

1: No comparison as managed somewhere else: Just show a read-only version of the most recent version eg: Non-comparable

2: Indicate something has changed: Don't show any changes but allow the user to link off to the files area in compare mode (possibly too hard) eg:

3: Show changes: Show visually what has changed, best effort in view eg. same asset but image has been replaced eg:

Updated designs can be found in the Styleguide

[chillu]

I think it's pretty rare that the associated file in the same "asset database record" is replaced (as opposed to authors just removing the asset record and adding a new one). So unless "3. Show changes" is easy to implement, I'd say we go for "2. Indicate something has changed".

[clarkepaul]

I would think that either opt 1 or 2 would be best, opt 1 being the easiest and probably sufficient as a first step (if time was limited) and opt 2 would be the best way to indicate that changes have been detected and allowing the user to go to the history of the file to inspect differences from there, in a detailed way.

There is another opt (4), and that would be to break open the file with all its fields in this view. Personally I don't like that approach though as it would very soon become to many fields in a single view. It also communicates that the file fields are managed on the page when their managed elsewhere.

[tractorcow]

Actually, even if you are changing the database record, it still makes a lot of sense to show a diff of the two files. E.g. ImageID changing from 3 to 5 would still make sense to show a side-by-side of the two versions of the two record IDs.

At the moment showing the diff between a switch between two image diffs is a terrible mash of string manipulation which does a string diff comparison of the two image titles. E.g.

-oldtitle
+newtitle

Code is in DataDifferencer.php

// Set the field.
$diffed->setField(
    $setField,
    DBField::create_field('HTMLFragment', Diff::compareHTML($fromTitle, $toTitle))
);

I actually would like 3. Images are comparable (show side by side) so I don't think 1 is quite correct.

[clarkepaul]

Maybe option 3 is the long goal using a simple view like what @tractorcow has suggested but for now can we just keep it at option 2?
We would then need to decide where that link goes.

[chillu]

@sachajudd Are you still working on the other field views for this? Specifically, GridField.

[sachajudd]

@chillu yes 🙂

[sachajudd]

Interactive design concepts can be found here for GridField and changes to sort order: https://projects.invisionapp.com/share/FTG16IYV5KB#/screens/287812383
Note: the entry link (3) should be "History tab compare mode - single"

"Compare Mode" for multiple GridField items (4) we have added states to indicate the comparison between the item's page status e.g. Draft → Published
We were discussing if the page status New → Draft is necessary or causes confusion.

Sort order is indicated in the “Pos” (Position) column e.g. 2 → 1
Switch between designs (3) and (4) to view sort order changes.

We were wondering the level of transparency that is possible to show within changes for a GridField item indicated by option 1 and 2 of the "Changes" column. Also if the "Last author" column is necessary.

Would be great to get some feedback on this 🙂

MultiselectField compare example

Designs can be found in the Styleguide

[robbieaverill]

Nice work @sachajudd, looking good overall! Here are my thoughts/comments (for the compare mode):

Personally I'd like to see the position indicators colour coded in the same manner as the rest of the changes, i.e. old position in red, new position in green.

What do the dates and times in the RECORD column indicate?

I find the use of the colour coding in the published states a little confusing. If red is always the old version and green is the newer version then that's fine, but it gets a little confusing (for me) when we have for example NEW (green) -> DRAFT (red) which I assume is indicating that the record was added in the new version and didn't exist in the old version. Maybe we could highlight the record text in green if it's entirely new, and red if it's been archived instead?

Could just be for the mockup (in which case ignore this) but most uses of GridField will probably be for objects that don't track the author. SiteTree does (via versioned), but generally people would use GridField for simpler data structures than that which probably won't have that column.

In my view it's important to replicate the GridField columns that are defined for the editable version of it. In that respect I don't think we should be adding columns like Changes in the example, or the Author columns (unless a dev has specified that they should be there). If we have a GridField for StaffMembers say (which is versioned), which has FirstName, Surname and Email as the columns, do we show the difference for all three summary field columns, or just the first (FirstName)?

For GridField's of unversioned data objects will we just show whether they've been added or removed?

There could be cases where a sort order is defined for non versioned dataobjects via a versioned many many through relationship (I think!?) so the sort order could be changed between page versions while the underlying data may not be, so that scenario is possible.

[chillu]

@sachajudd Agree with everything that Robbie said - can you please review, provide feedback and update designs?

In my view it's important to replicate the GridField columns that are defined for the editable version of it.

Agreed, those columns are usually chosen to make a record uniquely identifiable - they might all have the same value in a "title" column for example. I don't think showing inline changes for these columns is critical, as long as there's an indication that the record has changes in the first place, and you can click through to the individual record compare view. On that note, why isn't there a trigger action in each row for this?

Can you please capture all form field variations in one design doc? For example, we only have screenshots for UploadField here in the comment thread, which will make it harder to follow the designs later on.

@tractorcow Can you please look through this from a data model perspective, and check if we could implement this? I'm not sure about counting the number of changes, that gets pretty tricky with nested relationships, and is somewhat bound to us solving "activity feeds".

I've added a new AC:

• When viewing changes in a field with more detail (e.g. GridField), I can click through to the detailed compare view

[clarkepaul]

Cheers for the feedback, we'll make some amendments and @sachajudd will post back up. @chillu we've got all of the individual fields in readonly in the Style Guide but as for all fields in comparison were working through them—depending on how many there are we might be able to create a compiled list.

[clarkepaul]

If we have a GridField for StaffMembers say (which is versioned), which has FirstName, Surname and Email as the columns, do we show the difference for all three summary field columns, or just the first (FirstName)?

We only show the name/title, and only the newest variation of it so you don't see a comparison in the list view at all except the state and position change.

[sachajudd]

It gets a little confusing (for me) when we have for example NEW (green) -> DRAFT (red) which I assume is indicating that the record was added in the new version and didn't exist in the old version. Maybe we could highlight the record text in green if it's entirely new, and red if it's been archived instead?

@robbieaverill good point, I have added "NEW" and "DELETED" pills to the right-hand side in the "Changes" column. Would this make more sense?
(DRAFT is actually orange for future reference 🙂).

I've also created an example of an item that has been added back into the gridfield, Archived → Draft with an "ADDED" pill. Let me know if this scenario would work.

We have decided to leave the "POS" column without colour coding as with the numbers it become quite hard to read. But we are open to seeing how this will work in practice.

@ingo the gridfield would be it's normal unselected state (not read-only) so each item can be clicked into (I think this is what you mean by the trigger?), there is no icon as there would probably be no other actions except to view the item as far as I'm aware.

I have also added versioned/unversioned examples of the gridfield (4).

Updated designs and a walkthrough can be found here: https://invis.io/FTG16IYV5KB#/287812383_GridField_History_-_Menu

I hope to update the read-only form field variations throughout the week 🙂
I will although in the meantime add the form fields to the design menu until they are finalised.

Thanks for all the feedback, this has been a great issue to dive into!

[chillu]

Ingo: Agreed, those columns are usually chosen to make a record uniquely identifiable - they might all have the same value in a "title" column for example

Paul: We only show the name/title, and only the newest variation of it so you don't see a comparison in the list view at all except the state and position change.

I've realised that this table isn't defined by developers for their particular model, but rather a general purpose table that the CMS auto-creates. In this case, it might be prudent to default to showing DataObject::$summary_fields, which defaults to Name or Title fields (if they exist), but can be customised to show more fields if required to uniquely identify the record. No changes required to the design, just an implementation note.

there is no icon as there would probably be no other actions except to view the item as far as I'm aware.

Makes sense!

Sacha, you asked "What's the level of transparency possible for changes within each item?", in the context of the table design. The answer is "none", just to keep things simple from an implementation perspective. As long as we indicate that something has changed, it seems feasible to just point users to the detail representation of that particular record, right?

[sachajudd]

@chillu right, I agree! Designs have been updated 🙂

[chillu]

I've split out cards for ListboxField/MultiSelectField and UploadFIeld, and removed the related ACs from here: silverstripe/silverstripe-versioned-admin#31 and silverstripe/silverstripe-versioned-admin#32

[maxime-rainville]

We never got around to this. If we did now, we probably would have to start from scratch.