WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 5 years ago

#31196 closed enhancement (fixed)

Loading indicators for the Customizer preview

Reported by: celloexpressions Owned by: ocean90
Milestone: 4.2 Priority: normal
Severity: normal Version: 3.4
Component: Customize Keywords: has-patch
Focuses: ui, javascript Cc:

Description

Blank screen when the Customizer is first loaded looks broken, and nothing happening visually when following a link in the preview or when a change requires the refresh transport that takes a while looks odd. Let's make more liberal use of loading indicators and/or semi-transparent overlays.

Attachments (9)

31196.patch (2.4 KB) - added by Fab1en 5 years ago.
Display a loading indicator when user navigate around during preview
31196.2.diff (2.9 KB) - added by westonruter 5 years ago.
Additional change: https://github.com/xwp/wordpress-develop/commit/3786b03a1245c0540b06a29513b739aac17b7295
31196.3.patch (2.8 KB) - added by Fab1en 5 years ago.
Display a loading indicator when customizer is busy
31196.translation.patch (1.9 KB) - added by Fab1en 5 years ago.
Patch to apply on top of [attachment 31196.3.patch] for loading message translation.
31196.4.patch (4.2 KB) - added by Fab1en 5 years ago.
31196.3.patch + translation
31196.5.patch (3.1 KB) - added by Fab1en 5 years ago.
Move loading message translation to the right place
31196.6.patch (3.1 KB) - added by westonruter 5 years ago.
Additional change: https://github.com/xwp/wordpress-develop/commit/3695cac0671e6f3a0547aabcaf1d06542a28602d
31196.7.diff (3.8 KB) - added by westonruter 5 years ago.
Additional change: https://github.com/xwp/wordpress-develop/commit/a3556ae2f1563b47740fb6c200c324e76edefeb7
31196.8.diff (4.1 KB) - added by westonruter 5 years ago.
Additional change: https://github.com/xwp/wordpress-develop/commit/bde0642eb06249dcc02121873252d0c9f79824c9

Download all attachments as: .zip

Change History (31)

#1 @westonruter
5 years ago

+1

So there should be a loading indicator when the preview first loads, when the preview refreshes, and when navigating to another URL.

I implemented something for the latter situation of showing a loading indicator when navigating around the site in the preview (and this would also work when refreshing, if we send an unload message when the refresh logic is invoked):

In customize-controls.js, we can add:

wp.customize.previewer.previewUrl.bind(function () {
    wp.customize.previewer.send( 'unload' );
});

And then in customize-preview.js add:

wp.customize.preview.bind( 'unload', function () {
    jQuery( 'body' ).addClass( 'customizer-navigating-away' );
    jQuery( 'html' ).prop( 'title', 'Loading...' );
})

And then we can add this CSS to the Customizer preview:

body.customizer-navigating-away {
        transition: opacity 1s;
        opacity: 0.25;
        cursor: progress !important;
}
body.customizer-navigating-away * {
        pointer-events: none !important;
}

The result is that the contents of the Customizer preview fade to 0.25 opacity, a progress cursor appears along with a “Loading…” tooltip, and finally clicking is disabled on the preview until the page refreshes. Again, this is only addressing the UX issues when navigating around the site in the preview.

As an aside: transactions (#30937) will improve the loading experience since the iframe would be populated with the natural URL (#30028) as opposed to fetching the contents via Ajax and then document.write'ing them: the browser's normal loading indicators would appear.

Last edited 5 years ago by westonruter (previous) (diff)

#2 @westonruter
5 years ago

  • Focuses javascript added

#3 @DrewAPicture
5 years ago

If we could get a patch here, it would be good for 4.2 consideration.

@Fab1en
5 years ago

Display a loading indicator when user navigate around during preview

#4 @Fab1en
5 years ago

Here is a patch that puts @westonruter code in. I will try to extend it to also address refresh transport loading time.

#5 @westonruter
5 years ago

In 31196.2.diff, I also add a spinner to appear before the preview iframe loads for the first time. Patches are also added to this PR: https://github.com/xwp/wordpress-develop/pull/68/files

@Fab1en
5 years ago

Display a loading indicator when customizer is busy

#6 follow-up: @Fab1en
5 years ago

  • Keywords has-patch added; good-first-bug needs-patch removed

I added the loading spinner when a control with refresh transport is triggering a preview reload in 31196.3.patch. I also renamed the unload event to loading.

One thing is still missing : "Loading" message translation. Is it OK to introduce a new JS l10n global var to handle this in customize-preview.js ?

@Fab1en
5 years ago

Patch to apply on top of [attachment 31196.3.patch] for loading message translation.

This ticket was mentioned in Slack in #core-customize by drew. View the logs.


5 years ago

@Fab1en
5 years ago

31196.3.patch + translation

#8 in reply to: ↑ 6 ; follow-up: @westonruter
5 years ago

Replying to Fab1en:

I added the loading spinner when a control with refresh transport is triggering a preview reload in 31196.3.patch. I also renamed the unload event to loading.

One thing is still missing : "Loading" message translation. Is it OK to introduce a new JS l10n global var to handle this in customize-preview.js ?

It's ok, but why not add it to the WP_Customize_Manager::customize_preview_settings() method like so? https://github.com/xwp/wordpress-develop/pull/61/files#diff-4fb8a477f559bdfad2c1e6db6d1c8b06R832

#9 in reply to: ↑ 8 @Fab1en
5 years ago

Replying to westonruter:

It's ok, but why not add it to the WP_Customize_Manager::customize_preview_settings() method

Yes, you are right. I was looking for that settings variable without finding it.

@Fab1en
5 years ago

Move loading message translation to the right place

#10 @westonruter
5 years ago

  • Milestone changed from Future Release to 4.2
  • Owner set to ocean90
  • Status changed from new to reviewing

In 31196.6.patch, use better class name wp-customizer-unloading and add CSS transitions.

This ticket was mentioned in Slack in #core-customize by westonruter. View the logs.


5 years ago

#12 @westonruter
5 years ago

Key finding from @ocean90:

If you click on a linked image (href=src of image) the Customizer does a POST request to the image which produces a 405 Method not allowed error. Reproduced in 3.8 as well. But the issue is, that the loading overlay will remain on an error.

#13 @westonruter
5 years ago

  • Keywords commit added

In 31196.7.diff, send a message to the preview when a failure occurs to load a URL, and remove the loading indicator when this happens.

#14 @ocean90
5 years ago

  • Keywords commit removed

See https://wordpress.slack.com/archives/core-customize/p1424529431000739

The loading indicator added by /wp-admin/css/themes.css makes the Customizer unusable in IE11.

#15 @westonruter
5 years ago

Hummm. I downloaded a Modern.ie VM for IE11 on Windows 10, and I could not reproduce the issue: https://cloudup.com/ceWX2RRHMxa

The opening and closing of sections seems unaffected by the transitioned opacity in the preview.

User agent: Mozilla/5.0 (Windows NT 6.4; WOW64; Trident/7.0; .NET4.0E; .NET4.0C; rv:11.0) like Gecko

This ticket was mentioned in Slack in #core-customize by westonruter. View the logs.


5 years ago

#17 @celloexpressions
5 years ago

Need to resolve the performance issues here if we're going to make the fast-approaching enhancements deadline.

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


5 years ago

#19 @westonruter
5 years ago

  • Keywords needs-patch added; has-patch removed
  • Owner ocean90 deleted

Changing keyword from has-patch to needs-patch due to IE11 issue. I put the call for help out on #core.

#20 follow-up: @afercia
5 years ago

I'd check the spinner image used as background, I suspect this is not just an IE11 issue but it happens in all browsers depending on the hardware. If you have a powerful machine with a powerful GPU you won't probably notice performance issues, but on old machines without a GPU the spinner is always there.. spinning even if not visible. See screenshot, Chrome on an old machine: CPU at 97% is not just a "peak", it is always at 97% :)

https://cldup.com/2OjcRKZ8D6.png

#21 in reply to: ↑ 20 @westonruter
5 years ago

  • Keywords has-patch added; needs-patch removed
  • Owner set to ocean90

Replying to afercia:

I'd check the spinner image used as background, I suspect this is not just an IE11 issue but it happens in all browsers depending on the hardware. If you have a powerful machine with a powerful GPU you won't probably notice performance issues, but on old machines without a GPU the spinner is always there.. spinning even if not visible. See screenshot, Chrome on an old machine: CPU at 97% is not just a "peak", it is always at 97% :)

Great insight!

In 31196.8.diff: Remove hidden background image when iframe loads to help with performance.

#22 @ocean90
5 years ago

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

In 31697:

Customizer: Add loading indicators for the Customizer preview.

props Fab1en, westonruter.
fixes #31196.

Note: See TracTickets for help on using tickets.