WordPress.org

Make WordPress Core

Opened 4 months ago

Last modified 19 hours ago

#49288 reviewing defect (bug)

Metabox holders missing their border and "Drag boxes here" text

Reported by: xkon Owned by: audrasjb
Milestone: 5.5 Priority: normal
Severity: normal Version:
Component: Administration Keywords: has-patch has-screenshots
Focuses: ui, accessibility, administration Cc:

Description

Splitting this up from #39074. While editing Posts/Pages the advanced-sortables was missing a visible border and it was of 0px height so kind of hard to even drag a postbox there even if you knew it existed.

This was unfortunately extended into all metabox holders within the Edit so even if the sidebar was emptied there was no indication then that a draggable placeholder exists there.

I've managed to track down changes regarding this back to ticket #26399 & changeset 36295.

After some investigation, I also realized that the Drag boxes here text was also hidden from the Dashboard and all other areas for me. This was due to the @media rules pointing to specific ranges of a max 1800px.

Attachments (17)

49288.diff (2.3 KB) - added by xkon 4 months ago.
edit_before.jpg (120.3 KB) - added by xkon 4 months ago.
edit_after.jpg (129.5 KB) - added by xkon 4 months ago.
dashboard_before_over_1800wide.jpg (100.1 KB) - added by xkon 4 months ago.
dashboard_after_over_1800wide.jpg (102.4 KB) - added by xkon 4 months ago.
49288.2.diff (3.0 KB) - added by xkon 4 months ago.
49288.2_preview.gif (3.1 MB) - added by xkon 4 months ago.
49288 current patch.png (117.1 KB) - added by afercia 2 weeks ago.
49288 empty side sortable.png (79.2 KB) - added by afercia 2 weeks ago.
49288 dragging into side sortable.png (27.8 KB) - added by afercia 2 weeks ago.
49288.3.diff (7.3 KB) - added by afercia 12 days ago.
dashboard.gif (269.8 KB) - added by afercia 11 days ago.
post.gif (524.1 KB) - added by afercia 11 days ago.
49288 placeholder height.png (38.8 KB) - added by afercia 11 days ago.
Sortable placeholder height issue in Chrome when zooming.
dashboard-1200.gif (3.3 MB) - added by audrasjb 19 hours ago.
Dashboard looks good to me (viewport width 1200px)
Capture d’écran 2020-06-06 à 05.57.48.png (87.0 KB) - added by audrasjb 19 hours ago.
Edit post viewport width 1200px - no drag boxes here message
d08afd063388dfe33a59e49999b11474.mp4 (2.9 MB) - added by audrasjb 19 hours ago.
Attachement screen - works fine

Change History (40)

@xkon
4 months ago

#1 @xkon
4 months ago

  • Keywords has-patch needs-testing added

49288.diff adjusts the CSS rules and alters the postbox.js to be able to read all metabox holders to add the empty class.

Instead of the default 250px height (as seen on Dashboard) the Edit empty holders will have a 50px height to avoid a big blank space at the bottom of the page (i.e. for advanced-sortables). It also enables the Drag boxes here message for resolutions above 1800px.

@xkon
4 months ago

@xkon
4 months ago

#2 @xkon
4 months ago

  • Keywords has-screenshots added

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


4 months ago

#4 @xkon
4 months ago

  • Summary changed from Adjust the CSS/JS of the empty metabox holders to Metabox holders missing their border and "Drag boxes here" text

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


4 months ago

#6 @audrasjb
4 months ago

  • Keywords needs-design-feedback added
  • Milestone changed from Awaiting Review to 5.4
  • Owner set to audrasjb
  • Status changed from new to reviewing

#7 @afercia
4 months ago

@xkon thanks for the patch!

Re: "Drag boxes here": since we're here, it would be nice to make the text have a sufficient color contrast. The current grey #ccc has a very low contrast of 1.42:1 see https://jdlsn.com/color/?type=hex&color=cccccc&color2=f1f1f1. We could use #606a73 which is already used in the Dashboard for some elements.

Re: the "advanced" area: Seems to me the "regression" is a bit more... ancient 🙂. WordPress 3.0 used to have these styles to set a min-height on the sortable areas:

.inner-sidebar #side-sortables {
	width: 280px;
	min-height: 300px;
}

#post-body #normal-sortables {
	min-height: 50px;
}

#post-body #advanced-sortables {
	min-height: 20px;
}

So in WordPress 3.0 it was actually possible to move a meta box to the "advanced" area. The UI wasn't that great: there was no visible indication of the advanced area but it worked.

The "normal" and "side" min-heights are still used today. Instead, the "advanced" min-height was removed in [18975] see the related ticket #18314. This change seems completely unrelated to the original ticket purpose and probably not intended. Since then, it is not possible to move meta boxes to the "advanced" area. /Cc @azaozz may know more.

Worth also considering jQuery UI Sortable has a start event. By using it, it would be possible to add a CSS class to the body e.g. is-dragging-metaboxes and style anything in the page. This could help to highlight the available areas in some way. Then, remove the CSS class on the stop event which is the already used in postbox.js.

I do realize the legacy Edit Post page has a low priority now that there's Gutenberg but this is a small, long-standing, regression that can be quickly fixed. The dashboard issue instead needs to be fixed anyways :)

@xkon
4 months ago

@xkon
4 months ago

#8 @xkon
4 months ago

Thanks for going even further back and pinpointing a commit @afercia!

I was aware of the start() stop() but I didn't have an idea on how to use them. The color correction to #606a73 gave me the idea though that instead of having them hidden/visible on drag we could also make them even more visible and a11y friendly since the border is still at #b4b9be which isn't a good difference with the background also.

The changes in the 49288.2.diff are re-introducing the older min-height:20px on advanced-sortables as well as adjusting the Drag boxes here to permanently use #606a73.

Also adds/removes the is-dragging-metaboxes class on drag/drop which also adjusts the border color to #606a73 as well. This way we tackle both a visual hint and also making it compatible to standards.

Side note: I did also play around with fully hiding the sortables while empty and show them on drag but this was resulting either on "flickering" of the screen if the height was also changed ( i.e. with a display:none or height: 20px) or you just ended up having a huge gap at the end of the page that didn't look nice :-). That's why I ended up just bumping the border color to have the best of both.

Preview 49288.2_preview.gif

#9 @karmatosed
4 months ago

  • Keywords needs-design-feedback removed

From a design perspective, this doesn't seem to change the interface much, unless I am mistaken. As result removing the keyword as looks like it is an improvement fix.

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


4 months ago

#11 @afercia
4 months ago

This ticket was discussed during today's accessibility bug-scrub: the proposed patch appears to just need a final review.

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


4 months ago

#13 @afercia
4 months ago

  • Milestone changed from 5.4 to 5.5

This ticket was discussed during today's accessibility bug-scrub: With 5.4 Beta 3 approaching, agreed to move this ticket to the next release cycle.

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


3 weeks ago

#15 @afercia
2 weeks ago

  • Keywords needs-refresh added; needs-testing removed

Testing 49288.2.diff, the "drop area" below the post content is a bit too big, see attached screenshot. There are some subtleties in the way the edit post columns work and the way the columns behavior changed over time that make touching this code not that easy. Given that the classic edit post screen is legacy, I'd tend to think we should focus on a solution that is as simple as possible. On it.

#16 @afercia
2 weeks ago

RIght now, when the right column (#side-sortables) is empty (or contains only hidden meta boxes) it gets a thick dashed border. See screenshot attached below.

Last edited 2 weeks ago by afercia (previous) (diff)

#17 @afercia
2 weeks ago

When dragging a meta box inside the right column, the meta box placeholder has a thinner border. This way, the two different border styles help distinguishing the drop area from the placeholder. See screenshot attached below.

I'd tend to think the best option would be using the same pattern for the two drop areas below the post (#normal-sortables and #advanced-sortables) with a way smaller height and only while dragging.

Lastly, ensure all this doesn't break when the layout is set to 1-column under "Screen options".

Last edited 2 weeks ago by afercia (previous) (diff)

@afercia
12 days ago

#18 @afercia
12 days ago

  • Keywords needs-refresh removed

49288.3.diff builds on the previous patch and seeks to introduce a few improvements.

To recap, the main purpose of this ticket is:

  • on the Dashboard: display the "Drag boxes here" text also on large screens
  • on the edit post pages: give the ability to move the post boxes to all the available areas, including the "advanced-sortables" one which isn't possible since [18975]

Some testing would be nice. Please test at various resolutions and with the 2 column or 1 column layout, where applicable:

  • dashboard
  • edit post/page pages
  • edit attachment page

Details, TL;DR

  • adds a is-dragging-metaboxes class to the body when dragging
  • simplifies the JS and adds the is-dragging-metaboxes CSS class to all the sortables in the Dashboard and in the various edit posts pages
  • displays the CSS generated content "Drag boxes here" only on the Dashboard
  • since this text is meant to appear only on the Dashboard, moves the related CSS to dashboard.css
  • makes sure the text is displayed also on large screens with a viewport larger than 1800 pixels
  • better scopes the CSS differentiating between the one for #dashboard-widgets and the one for #post-body
  • removes the CSS rulesets related to .inner-sidebar and .has-right-sidebar as they're no longer used since WordPress 3.4, see [20272] / #20015

Visual changes for the Dashboard:

  • when dragging, the "empty" drop areas get a darker border
  • note: the empty areas are the ones that are really empty or that contain only hidden boxes

Visual changes for the edit post pages:

  • when dragging, adds a visual indication of all the drop areas, making it possible to move post boxes also to the "advanced-sortables" area
  • to respect the original implementation intent, there's no change in the 1 column layout

Some relevant history related to columns and sortables:
[18975] / #18314 / [20272] / #20015

#19 @afercia
11 days ago

See below an animated GIF to illustrate the behavior on the Dashboard.

Question: would it be preferable to always show the drop areas borders while dragging? The fact that borders are only shown for empty containers is a bit odd to me.

@afercia
11 days ago

#20 @afercia
11 days ago

See below an animated GIF to illustrate the behavior on the Edit post screen.

@afercia
11 days ago

#21 @afercia
11 days ago

Aside: while working on this I noticed a pre-existing issue.

With Chrome, when zooming out or in, at some zoom levels the "sortable placeholder" (the rectangle with a thin dashed border) has no height. It just shows like a doubled dashed line. See screenshot below.

When the placeholder height isn't explicitly set via CSS, jQuery UI sortable is supposed to compute a height based on the dragged element. See https://github.com/jquery/jquery-ui/blob/1.11.4/ui/sortable.js#L816-L817
This is a good thing because the placeholder will adapt to the post box collapsed / expanded height.

However, seems this computation fails because Chrome returns a height value which is 0 with some decimals e.g. 0.02083400000000002 thus when the Sortable code checks for ! element.height(), it fails. I guess jQuery UI sortable should prevent browser roundings by using parseInt() on all those values. Thinking this should be reported upstream. Although WordPress is using a pretty old version of jQuery / jQuery UI, this issue doesn't seem fixed on latest master as well.

@afercia
11 days ago

Sortable placeholder height issue in Chrome when zooming.

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


32 hours ago

@audrasjb
19 hours ago

Dashboard looks good to me (viewport width 1200px)

@audrasjb
19 hours ago

Edit post viewport width 1200px - no drag boxes here message

#23 @audrasjb
19 hours ago

@afercia

Question: would it be preferable to always show the drop areas borders while dragging? The fact that borders are only shown for empty containers is a bit odd to me.

Yes, it looks odd to me as well. I'm in favor of always showing the drop area borders when dragging.

Otherwise, the patch works fine on my side, great work :)

@audrasjb
19 hours ago

Attachement screen - works fine

Note: See TracTickets for help on using tickets.