WordPress.org

Make WordPress Core

Opened 8 months ago

Closed 6 months ago

Last modified 6 months ago

#46960 closed defect (bug) (fixed)

Site Health: Table design issue in small devices(iphone 5/SE).

Reported by: immeet94 Owned by: afercia
Milestone: 5.2.2 Priority: normal
Severity: normal Version: 5.2
Component: Site Health Keywords: has-screenshots site-health has-patch
Focuses: ui, accessibility, administration Cc:
PR Number:

Description

Table does not looks good under accordion in Site Health Info section for small devices. I have attached screenshot.

Attachments (15)

site_health_table_issue_iphone5_se.png (52.3 KB) - added by immeet94 8 months ago.
Please see this file for the issue.
#46960.diff (331 bytes) - added by jankimoradiya 8 months ago.
site-health-table-issue-updated.png (42.0 KB) - added by jankimoradiya 8 months ago.
#46960-1.diff (352 bytes) - added by immeet94 8 months ago.
update patch
46960.diff (608 bytes) - added by desrosj 7 months ago.
#46960-2.diff (391 bytes) - added by immeet94 7 months ago.
Please see this file for updated patch.
Screenshot.png (633.5 KB) - added by immeet94 7 months ago.
46960 non breakable content.png (243.8 KB) - added by afercia 7 months ago.
#46960-shashank-update.patch (1.1 KB) - added by shashank3105 7 months ago.
Patch file for this issue
contribute.local_wp-admin_site-health.php_tab=debug(iPhone 6_7_8)-before.png (88.0 KB) - added by shashank3105 7 months ago.
contribute.local_wp-admin_site-health.php_tab=debug(iPhone 6_7_8)-after.png (69.5 KB) - added by shashank3105 7 months ago.
46960.2.diff (536 bytes) - added by chetan200891 7 months ago.
Updated patch to fix table layout for small devices.
46960.3.diff (563 bytes) - added by chetan200891 7 months ago.
Updated patch to fix table layout for small devices. (Missed box-sizing)
46960.4.diff (643 bytes) - added by chetan200891 7 months ago.
Updated patch.
46960.5.diff (678 bytes) - added by afercia 7 months ago.

Download all attachments as: .zip

Change History (51)

@immeet94
8 months ago

Please see this file for the issue.

#1 @immeet94
8 months ago

  • Component changed from Help/About to General

#2 @Clorith
8 months ago

  • Milestone changed from Awaiting Review to 5.2.1
  • Severity changed from major to normal

Tables are not the best on responsive designs in the first place.

I'm not sure this is worth putting too much effort into for 5.2, as it's not a general page for casual browsing in the first place.

I'd be happy looking to tackle this for 5.2.1 if anything, where we can look at adding some responsive styling for tables, but with just two columns, you are very much controlled by the content of the columns them selves. We should try to take care so we don't disrupt the data or misrepresent it after all. One idea may be to add overflow styles for the x-axis to allow scrolling within the table on smaller devices.

#3 @Clorith
8 months ago

  • Keywords site-health added

#4 @jankimoradiya
8 months ago

Hello @immeet94,

I have solved above issues by adding css which is mention in #46960.diff file.

Please review #46960.diff file.

Thanks

#5 @SergeyBiryukov
8 months ago

  • Component changed from General to Administration

@immeet94
8 months ago

update patch

#6 @immeet94
8 months ago

Hi @Clorith

i follow your suggestion and create patch so please review it.

Thanks.

#7 @immeet94
8 months ago

Hello @jankimoradiya

i try you patch and it's working fine for me. Thanks for help !!

Thanks

#8 @immeet94
8 months ago

  • Keywords has-patch added; needs-patch needs-design-feedback removed

This ticket was mentioned in Slack in #core by immeet94. View the logs.


8 months ago

This ticket was mentioned in Slack in #accessibility by immeet94. View the logs.


8 months ago

#11 @afercia
7 months ago

#47179 was marked as a duplicate.

@desrosj
7 months ago

#12 @desrosj
7 months ago

  • Keywords needs-testing added

I tested #46960-1.diff and #46960.diff, but they did not seem to fix the problem for me.

46960.diff copies over the patch that @shashank3105 created on #47179. I have better results testing this patch.

@immeet94
7 months ago

Please see this file for updated patch.

@immeet94
7 months ago

#13 @immeet94
7 months ago

Hello @desrosj

I update patch and its works for me,so please check the patch and test it.

Thanks.

#14 @afercia
7 months ago

One idea may be to add overflow styles for the x-axis to allow scrolling within the table on smaller devices.

Not sure that would be a great user experience, as scrolling horizontally is very annoying on mobile. Not to mention it's extremely difficult for keyboard users and many screen reader users use a keyboard when using their mobile devices.

Worth noting the root cause of the issue is not just the use of tables. Non-breakable words and long URLs/paths make the table cells wider. See attached screenshot on the left.

Even using something like word-break: break-all; wouldn't help so much because the content would be mostly unreadable. See screenshot on the right.

I'd tend to think the only option is to transform the table layout in vertically stacked blocks. From a semantics / accessibility perspective there wouldn't be loss of information because these tables are only used for layout and have an ARIA role="presentation".

#15 @afercia
7 months ago

  • Keywords needs-refresh added; needs-testing removed

#16 @desrosj
7 months ago

I also wanted to call out that there are implications for CJK (Chinese/Japanese/Korean) languages with different word-break values that we need to consider.

#46960-2.diff also did not fix the issue for me. I found inconsistent documentation for break-word as a value for word-break. My testing was in Firefox. break-all was the only way to fix the issue consistently, but as @afercia noted, this is not the best experience as many times it would render the content unreadable.

#17 @shashank3105
7 months ago

Hello @desrosj and @afercia,

I have updated code for this issue and work for me. So can you please check my revised code in attached patch file and screens.

Thanks,
Shashank

@shashank3105
7 months ago

Patch file for this issue

#18 @Clorith
7 months ago

  • Milestone changed from 5.2.1 to 5.2.2

#46960-shashank-update.patch moves everything to be multiple lines regardless of the viewport size, this isn't a good user experience.

As @desrosj mentions, word-break can do weird things for various locales, so let's avoid that.

Are there any major issues with taking the overflow-x: auto approach? (This essentially lets you scroll horizontally if it overflows, the only downside is that there's no scrollbar to indicate this possibility on handheld devices, but I believe it's so ingrained that you can swipe sideways to read remaining text on these that that may not be an issue?)

#19 @afercia
7 months ago

Are there any major issues with taking the overflow-x: auto approach?

See comment:14. Also, as you point out (and depending on user settings) on some OSes there's no visual indication something can be scrolled horizontally.

This ticket was mentioned in Slack in #design by spacedmonkey. View the logs.


7 months ago

#21 @karmatosed
7 months ago

  • Keywords needs-design-feedback added

@chetan200891
7 months ago

Updated patch to fix table layout for small devices.

@chetan200891
7 months ago

Updated patch to fix table layout for small devices. (Missed box-sizing)

#22 @afercia
7 months ago

Thanks all for the patches, they seem a good start to me. Worth noting some long URLs and paths break in a new line only because they contain a hyphen. When they don't, long, unbreakable lines still overflow from the table:

http://cldup.com/2NAD6YyU8h.png

There are inconsistencies in the way browsers support CSS properties to break long lines. More importantly, seems some of these CSS properties don't work for table cells.

Quickly testing in Chrome, seems to me there's the need to:

  • set table-layout: fixed; on the table
  • set word-wrap: break-word; on the table cells

Some testing on other browsers would be nice :)

Also noting this ticket still needs some design feedback /Cc @hedgefield @Clorith

@chetan200891
7 months ago

Updated patch.

#23 @chetan200891
7 months ago

  • Keywords needs-refresh removed

@afercia Thanks for your suggestion. I have updated patch 46960.4.diff. I have tested in Chrome, Firefox and Safari on Mac OS. Looks good to me.

#24 @hedgefield
7 months ago

  • Keywords needs-design-feedback removed

Sounds like a good first step to me; complex tables just aren't easy to do on mobile. Shall we go with the latest patch? We can iterate afterwards and try more extensive tweaks to the table if need be.

#25 @desrosj
7 months ago

  • Keywords commit added

Thank you, everyone, for your work on this!

This looks good in my testing. Marking this as commit, but I'd just like to get @afercia's sign off as well.

#26 @audrasjb
7 months ago

Hi,I tested this one on my side and it works well. I think this one is good to go orf 5.2.2.

#27 @chetan200891
7 months ago

Just a thought, how about removing bottom-padding for first-child td?

This ticket was mentioned in Slack in #core by audrasjb. View the logs.


7 months ago

@afercia
7 months ago

#29 @afercia
7 months ago

Looks good to me, thanks all! Just a quick note: although the CSS coding standards mention 700 as value for the bold font weight, core uses 600 and for good reasons. See for example #43897, [37740], #36753. The coding standards should be updated.

@chetan200891 padding-bottom: 0; for the first child cell makes sense, it improves readability by connecting visually the related cells. Updated in 46960.5.diff.

#30 @afercia
7 months ago

  • Owner set to afercia
  • Resolution set to fixed
  • Status changed from new to closed

In 45463:

Administration: Improve the Site Health tables layout for small screens.

Props immeet94, jankimoradiya, desrosj, shashank3105, chetan200891, Clorith, hedgefield.
Fixes #46960.

#31 @afercia
7 months ago

  • Keywords fixed-major added; commit removed
  • Resolution fixed deleted
  • Status changed from closed to reopened

Reopening for 5.2.2 consideration.

This ticket was mentioned in Slack in #core by audrasjb. View the logs.


6 months ago

#33 @audrasjb
6 months ago

Hi @afercia do you think we can have a backport for this ticket today so we can ship that on 5.2.2 RC1?
Thank you! The RC processing is going to start at 19:00 UTC.

#34 @afercia
6 months ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 45514:

Administration: Improve the Site Health tables layout for small screens.

Props immeet94, jankimoradiya, desrosj, shashank3105, chetan200891, Clorith, hedgefield.

Merges [45463] to the 5.2 branch.
Fixes #46960 for 5.2.2.

#35 @afercia
6 months ago

  • Keywords fixed-major removed

#36 @spacedmonkey
6 months ago

  • Component changed from Administration to Site Health
Note: See TracTickets for help on using tickets.