Problem/Motivation

I noticed an UX regression in the File and Image Widget. The table header is always visible. The bug was introduced in #1898070: file.module - Convert theme_ functions to Twig Commit a4a887ac.

This is the related hunk from commit a4a887ac function template_preprocess_file_widget_multiple.

-  $output = empty($rows) ? '' : drupal_render($build);
-  $output .= drupal_render_children($element);
-  return $output;
+  $variables['element'] = $element;

I think should keep the previous behaviour.

HEAD without patch

HEAD, without patch

HEAD with patch

With patch

Beta phase evaluation

Reference: https://www.drupal.org/core/beta-changes
Issue category Bug, this is a regression.
Issue priority Normal because it doesn't break anything, just looks broken.
Disruption No disruption.

Proposed resolution

Only display the table header when there are files.

Remaining tasks

None at this time.

User interface changes

The previous behaviour will be restored: The table header will only be displayed when there are files.

API changes

n/a

CommentFileSizeAuthor
#5 2457793-4.patch2.25 KBclaudiu.cristea
#5 2457793-4-test-only.patch1.85 KBclaudiu.cristea
remove-empty-table-file-widget.patch405 byteswebflo
file-widget-head.png91.78 KBwebflo
file-widget-fix.png78.42 KBwebflo

Comments

webflo’s picture

webflo commented
Status: Active » Needs review
star-szr’s picture

star-szr
he/him
Primary language English
commented

Good catch @webflo! Code looks good, should we add a small test?

rogerlenoir’s picture

rogerlenoir commented

Thanks webflo. The patch still applies and solves the problem.

rogerlenoir’s picture

rogerlenoir commented
claudiu.cristea’s picture

claudiu.cristea
Primary language Romanian
Location Arad 🇷🇴
commented
StatusFileSize
new 2457793-4-test-only.patch1.85 KB
new 2457793-4.patch2.25 KB

Added also the test. There's "test only" patch proving the regression.

The last submitted patch, 5: 2457793-4-test-only.patch, failed testing.

star-szr’s picture

star-szr
he/him
Primary language English
commented
Issue summary: View changes
Status: Needs review » Reviewed & tested by the community

Looks good, has tests, thanks! Filled out the issue summary and added a beta evaluation.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Fixed

This issue is a normal bug fix, and doesn't include any disruptive changes, so it is allowed per https://www.drupal.org/core/beta-changes. Committed c2b35b0 and pushed to 8.0.x. Thanks!

  • alexpott committed c2b35b0 on 8.0.x 
    Issue #2457793 by claudiu.cristea, webflo: UX Regression in File- and...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.