WordPress.org

Make WordPress Core

Opened 6 years ago

Closed 6 years ago

#28761 closed defect (bug) (fixed)

wpviews: gallery shortcodes displaying "undefined" for an instant

Reported by: iseulde Owned by: azaozz
Milestone: 4.0 Priority: normal
Severity: normal Version: 3.9
Component: TinyMCE Keywords: has-patch
Focuses: Cc:

Description

We should return ''; while loading the gallery.
But ideally, we should create a loading function that inserts some content, because doing an ajax call to render a view is not uncommon. The embed view does the same... Something like this.loading() maybe?

Attachments (3)

28761.patch (800 bytes) - added by iseulde 6 years ago.
28761.2.patch (841 bytes) - added by azaozz 6 years ago.
28761.3.patch (3.4 KB) - added by azaozz 6 years ago.

Download all attachments as: .zip

Change History (8)

@iseulde
6 years ago

#1 @iseulde
6 years ago

  • Keywords has-patch added

This will make sure getHtml actually returns something.

@azaozz
6 years ago

#2 @azaozz
6 years ago

How about we insert a temporary (loading) placeholder while the data is being fetched? We can style it any way we want depending on the view, hide it completely, etc.

#3 follow-up: @helen
6 years ago

I thought we had discussed a loading indicator at some point - we should probably do that.

@azaozz
6 years ago

#4 in reply to: ↑ 3 @azaozz
6 years ago

Replying to helen:

I thought we had discussed a loading indicator at some point

Right. In 28761.3.patch:

  • Add loadingPlaceholder() that returns the html. Would it be better if this was a template?
  • Fix setContent(), it should empty the element before appending the new node.
  • getHtml() should always return a string.

The default loadingPlaceholder has the dashicons-admin-media icon and a small "loading" CSS animation. Does it need to be more "distinct" or more styling? (To test it: edit line 50 in mce-view.js, from var html = this.getHtml() || this.loadingPlaceholder(); to var html = this.loadingPlaceholder();.)

#5 @azaozz
6 years ago

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

In 29019:

TinyMCE wpView:

  • Add loadingPlaceholder() that returns some html to use as placeholder while the view is loading. Includes a subtle CSS based loading animation.
  • Fix setContent(), it should empty the element before appending the new node.
  • Change getHtml() to always return a string.

Fixes #28761.

Note: See TracTickets for help on using tickets.