Switch the recent content block to use a list instead of a table, matching recent comments block

Here's a first stab. Might need some styling as well.

(Merging "node system" and "node.module" components for 8.x; disregard.)

Sorry for the noise. Looks like the component is stored as a string value so we'll need to update these nodes programmatically before we rename the component.

Maybe better to close this one as duplicate of #2020393: Convert "Recent content" block to a View to focus on changes as we do with #849670: Recent content block usability

Tags

@dawehner - which Views patch is this postponed for?

I think it was #2020393: Convert "Recent content" block to a View, but @andypost suggests we close this in favor of #849670: Recent content block usability

Thanks for the context @tim.plunkett.

I just noticed that #849670: Recent content block usability was marked a duplicate of this and then got switched to Needs Work after a test failed, so I changed it back to a duplicate. I guess whoever wants to work on it can pick which issue to use.

Patch attached.

Changes to core/modules/node/config/optional/views.view.content_recent.yml
I have to be honest I don't actually know how to restore to the view yaml state so I just reinstalled to test that the changes take effect.

Current

Proposed

Update with test changes.

Code looks good. Code accomplishes goals specified.

Just a re-roll with #17.2 addressed.

Patch does what it sets out to do, issues that @dawehner pointed out in #17 are addressed.

Reviewing this.

Thanks for the screenshots and the clear steps to reproduce!

So this isn't a bug exactly unless there is a specific accessibility violation -- and if there is such a bug, it's a bug with Views tables everywhere. However, switching this from a table to a list by default should improve both usability and accessibility, so per https://www.drupal.org/core/beta-changes, this is a good change to complete during the Drupal 8 beta phase.

I tested the patch manually, and I think that the formatting of the list could use a little more work.

1. For one thing, it's now essentially a bunch of links in a row with no context, which as far as I know isn't so good for accessibility in itself.
2. We're converting a table view to a list view, which means giving up the table headers, responsive columns, and the compact way the table is scannable, but not making any changes to the fields or display of the labels. So it essentially just becomes an underpowered table in a <ul>.
3. It doesn't look all that great. Here's the block in the right sidebar in Bartik:
   
   Four lines for every bullet is kind of difficult to scan and understand; we should put it on one line or at most two. Compare with the recent comments block:
   
4. I think the author name field should be rewritten to include "by [username]" or something; otherwise it's difficult to understand what the username link is.
5. Do we still want the admin operation links here for a sidebar block? The recent comments block doesn't have them, for example.

 

So let's do some work on the view to improve these things. When a new patch is ready, please add some updated screenshots, and tag it "Needs usability review" to get feedback from a usability maintainer. Also, let's document the beta evaluation in the summary while we're at it. Thanks!

Changed the fields to inline. Wanted to change username to an address tag, but that removed inline; Inserted the "by" text. Added a space separator. Needs a general opinion before asking for a usability review.

From reading more in the duplicate issue, we want to also remove the edit/delete actions, because this used to be a "manage recent content" block, meant for the dashboard.

Better screenshot in sidebar:

This patch no longer apples.

Are we going to have to bump this to 8.1?

Re-rolling patch. Got error while trying create interdiff.

1 out of 5 hunks FAILED -- saving rejects to file /tmp/interdiff-1.ucBEbT.rej
interdiff: Error applying patch1 to reconstructed file

Fixing failing test.

The patch is looking good now. The screenshots from #23 look the same as the patch I just applied.

Hopefully we can get this into Core as it looks way better than the default table version.

@xjm how is our timing?

This looks good to me for accessibility, although it isn't really an accessibility problem with the table at this point. More of a UX issue as far as I'm concerned.

Oh ya, here is a screenshot before the patch.

This block dates back to the Dashboard module, which we removed earlier in the D8 cycle. I think the accessibility claims in the issue summary are pretty spurious TBH, but I can see this going in during beta in terms of post-Dashboard clean-up, especially given admin/content is also now a View, and we don't need two similar-but-different admin views for content.

Making this block match the Recent Comments block in presentation makes a lot of sense, and is what I think users would expect given the similar names of the blocks. However, that's not what the patch currently does:

IMO we should remove the "Delete" link and "by author" field here and add "time ago" to make it match the output Recent comments. Then if we want, we could always file a follow-up issue to add "by author" to both blocks.

Agreed with dawehner on the rationale for not requiring an upgrade path here, so once we get that small tweak made, I think this is good to go. Also, please update the screenshot in the issue summary to reflect the current patch.

Oops. Forgot to upload the screenshot of the two blocks side-by-side.

Another silly thing is the "More" link showing up when there are no "More" records, so let's get that killed as well.

Here is updated patch.

Added missing updated test.

Sharique, can you explain the changes in #36, and/or show an updated screenshot?

@lokapujya changed the view as per comment from @webchick.

Thank You for the update.

So, I think Sharique fixed: remove the "Delete" link and "by author" field here and add "time ago".

What about the More link? Are we going to fix that?

Actually "More" points to admin/content and shows all content, so maybe it's ok? I think most sites are going to have "more" content.

The more link is fine, but it should only show up when there are > 10 (or whatever the block is set to show) pieces of content.

In other words, we need to turn the "Always show more link" setting to false.

Yeah, but how is "Recent Content" going to be used; Is it still for admins or did we just repurpose it (when we removed the delete link.)? Because regular users won't have access to admin/content.

Actually, I don't know if it's original purpose was for admins, I just deducted that from finding out that it was on the dashboard and seeing that it had delete links. But, I just tested it on simplytestme as an authenticated user and the More link shows up; Then you get access denied when you click on it.

Oh, right. In that case, let's remove the more link. I don't think recent comments has one, either, and the point is to make them consistent. If admins need to get to the content admin page, they can easily get there from the toolbar.

Updated patch with more link removed.

Ahh, I forgot to update tests. Here is patch with updated tests, it no longer checks for More link.

Patch applies nicely and works great. Thanks!
The more link is gone.

@dawehner, The link to 'admin/content' not accessible to normal users, so it does not make sense to have link there, opening link will give them access denied, which in not good from usability perspective.
Second pt, we have to make is similar to "Recent comments" block, which also don't have "More" link, so removing more link make sense.

Right. Back in the day, the link to admin/content made sense, since this was a block specifically targeted toward the admin dashboard. However, Dashboard got removed, and furthermore if you want an admin block, you can either repurpose your own or make a totally new one since Views is now in core. :)

Yay!

Committed and pushed to 8.0.x. Thanks, all! Much better. :)

LOL