Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#12755 closed New Feature (wontfix)

Provide an API to set editor as busy

Reported by: Marek Lewandowski Owned by: Marek Lewandowski
Priority: Normal Milestone:
Component: General Version:
Keywords: Cc:

Description

Overview

During mediaembed plugin implementation I needed a nice way to indicate that external resource is loading.

Presentation

I out with indicating that by changing the mouse cursor to wait icon.

But it's still not enough. We need more than that because we can't rely only on mouse cursor (mobile?).

I think that we might small spinning circle in right bottom corner of toolbar area or maybe in a corner of the viewport.

API Implementation

We must be sure that multiple calls can occur from different plugins, and busy state will be ended only if all of them will unset busy state.

We might implement it in similar fashion as deferring in Bender (https://github.com/benderjs/benderjs/issues/141). So here we should favor returning a callback rather than using a counter internally.

// From now developer is obligated to call releaseBusy().
var releaseBusy = editor.setBusy();

var myAsyncFunction = function() {
	// Do my stuff.
	releaseBusy();
};

var myAsyncError = function() {
	// Oopsy daisy, something bad happened.
	releaseBusy();
};

Bad Usage

Are we concerned about plugin poor implementation? Therefore not calling the function? If so we might restrict by default maximum "lifespan" for one callback eg. to 30s.

Change History (9)

comment:1 Changed 4 years ago by Marek Lewandowski

Status: newconfirmed

Let's do it as an external plugin, it will be named busyindicator.

Last edited 4 years ago by Marek Lewandowski (previous) (diff)

comment:2 Changed 4 years ago by Marek Lewandowski

Milestone: CKEditor 4.5.0

comment:3 Changed 4 years ago by Marek Lewandowski

Owner: set to Marek Lewandowski
Status: confirmedreview

Changes as opposed to original mediaembed implementation:

In case of inline we decided to change cursor only within editable rather than host html element.

Pushed to t/12755.

comment:4 Changed 4 years ago by Marek Lewandowski

Manual tests reduced, added a method _getHost that returns a host element.

Repushed to t/12755.

comment:5 Changed 4 years ago by Piotrek Koszuliński

Status: reviewreview_failed
  • The manual tests should be in plugindir/manual/ directory (and of course since one test file is named inline, the second would need to be named classic).
  • At least on IE8 there's problem with applying the waiting indicator. In the inline editor it doesn't work at all and on framed one it is ok only when cursor is outside body. Either selectors have to be fixed or it must be explained in the test.

comment:6 Changed 4 years ago by Marek Lewandowski

tl;dr

It seems to be IE limitation that you can't change cursor within contenteditable.

I think that this plugin is useless if it can't reliably work on IEs (since it's a big part of our end-users).

I've created an issue in connect platform.

/tl;dr

Problem with cursor in IE

It looks like it's impossible to change cursor within contenteditable in IE.

There were some hacks mentioned in the net, like:

But none of these were working as expected.

It wasn't event possible to override it using directly <p style="cursor: pointer !important">foo</p>, that should have the highest priority, or using custom .cur file.

Also at some point I thought that hacky CSS property might cause a problem, since it's detected as invalid propert in newer IE, but still the issue is with pure contenteditable.

Proposed solution

We need to deliver a solution that will work for all UAs.

We can indicate a busy state by placing a spinner in toolbar, however this raises some concers, like buttons alignment, and we need consider floating toolbar.

Showing indicator in toolbar would cover portability (mobile, no cursor) and accessibility issues.

We might enable cursor only on non-IE browsers, since it's a great addition.

comment:7 Changed 4 years ago by Marek Lewandowski

Ok so we decided to use notification plugin for that purpose.

For that we'll create extra class that will handle related/grouped notifications.

It's important for this class to:

  • Have simple interface.
  • Allow to override behavior when all "threads" are finished.

comment:8 Changed 4 years ago by Marek Lewandowski

Resolution: wontfix
Status: review_failedclosed

Concept of busyindicator plugin has been dropped in favor of notification-based type.

You can follow discussion at #12810.

comment:9 Changed 4 years ago by Marek Lewandowski

Milestone: CKEditor 4.5.0
Note: See TracTickets for help on using tickets.
© 2003 – 2019 CKSource – Frederico Knabben. All rights reserved. | Terms of use | Privacy policy