Allow custom navigation feedback

[marc-mabe]

Bug description

Unpoly adds the class up-loading on target elements that are currently loading.
I need to add a loading spinner in this case but not on this element but on another element before that I marked with up-loading-spinner.

With the following script I want to accomplish that but it does not get triggered if unpoly adds the up-loading class. It gets triggered if I add the class manually but obviously this does not help.

Reproduction project

https://glitch.com/edit/#!/orchid-placid-garden
This shows what I want to accomplish but I can't emulate a late server response (or I don't know how) so I manually added an up-loading to the table target.
You should see the spinner on the table as expected but if unpoly adds the up-loading class the compiled handler is not triggered.

Steps to reproduce the behavior:

1. Go to reproduction project linked above
2. see expected result while loading
3. remove up-loading on table element
4. emulate slow server
5. click on #table1
6. see no spinner shown even if up-loading was added (and removed) by unpoly

Expected behavior

If unpoly adds up-loading to an element compiled handlers targeting this element should be triggered and cleaned up on removing up-loading again.

Browser version

• OS: Linux]
• Browser Chrome, Firefox

[triskweline]

We generally don't support compilation for temporary state classes like .up-loading or .up-currrent.

A compiler/destructor would also not be the best tool here, as there are cases where .up-loading is removed, but the fragment is not swapped. E.g. when the request is aborted or when the server sends different HTML.

What I think we should support is events like up:feedback:start and up:feedback:stop so users can implement more complex loading states.

[marc-mabe]

We generally don't support compilation for temporary state classes like .up-loading or .up-currrent.

OK got it - would be great to have this documented on the compiler page

A compiler/destructor would also not be the best tool here, as there are cases where .up-loading is removed, but the fragment is not swapped. E.g. when the request is aborted or when the server sends different HTML.

That's what I expect as it's not loading anymore - the loading indicator should disappear as soon as it's not loading (or processing) anymore. The result of that is a different question and should be handled differently.
(I hate apps not removing loading spinners on errors forcing you to hard reload and loose all state you had before)

What I think we should support is events like up:feedback:start and up:feedback:stop so users can implement more complex loading states.

I'm not sure what "feedback" means as it's a very general term not specific to loading/processing something.

[triskweline]

I'm not sure what "feedback" means as it's a very general term not specific to loading/processing something.

Unpoly uses the { feedback } option to enable up-active and up-loading classes. See navigation feedback.

[marc-mabe]

I'm not sure what "feedback" means as it's a very general term not specific to loading/processing something.

Unpoly uses the { feedback } option to enable up-active and up-loading classes. See navigation feedback.

Would it be possible to take action on the following:

• List of targets going to be replaced
• What is the source element
• What action has been triggered

[marc-mabe]

Hi @triskweline,

After wrapping my head about this once again ... I think it would be enough to support a different target for up-feedback to set the up-loading class.

E.g.: Define custom feedback target instead of boolean to up-feedback. This would add the up-loading CSS class to #my-loading-indicator instead of the targeted fragment(s) (#my-target in this case).

<div>
    <div id="my-loading-indicator">...</div>
    <div id="my-target">...</div>
    <a href="/bar" up-follow up-target="#my-target" up-feedback="#my-loading-indicator">Bar</a>
</div>

What do you think?

[triskweline]

Hi @marc-mabe. Thanks for thinking about this some more.

While I like the minimalism of this, here's one concern:
The [up-feedback] attribute controls two classes: .up-active (for the element causing the request) and .up-loading (for the fragments being updated). If we allow a selector here, we need a good (= works for most users) opinion about which class this controls.

One advantage of an event would be to delegate this opinion to individual user apps.

[marc-mabe]

Hi @marc-mabe. Thanks for thinking about this some more.

While I like the minimalism of this, here's one concern: The [up-feedback] attribute controls two classes: .up-active (for the element causing the request) and .up-loading (for the fragments being updated). If we allow a selector here, we need a good (= works for most users) opinion about which class this controls.

One advantage of an event would be to delegate this opinion to individual user apps.

Hi @triskweline,
I do see it this way:

• .up-active controls the source element, The element causing that event, which is defined and does not make sense to be a different one.
• This leaves us with .up-loading for the target element.
  • the target can be controlled already with up-target which is also used by .up-loading
  • .up-loading makes sense to be on the target by default but a loading indicator is not the same functionality as the purpose of the targeted fragment.
  • Being able to use a different element for via up-feedback={selector} is an opt-in. So you have to explicitly define (or configure) it to be used this way.