OfficeDev / office-js

A repo and NPM package for Office.js, corresponding to a copy of what gets published to the official "evergreen" Office.js CDN, at https://appsforoffice.microsoft.com/lib/1/hosted/office.js.
https://learn.microsoft.com/javascript/api/overview
Other
683 stars 95 forks source link

Office.addin.showAsTaskpane does not work until event.completed() is called #3250

Open wh1t3cAt1k opened 1 year ago

wh1t3cAt1k commented 1 year ago

Summary

In Office event handlers (such as ribbon button handlers), Office.addin.showAsTaskpane does not work until event.completed() is called.

Your Environment

Expected behavior

Office.addin.showAsTaskpane() should always work, even inside asynchronous handlers for Ribbon buttons.

Current behavior

Office.addin.showAsTaskpane does not work until event.completed() is called.

Steps to reproduce

Our handlers look like this:

        Office.actions.associate(name, async event => {
            try {
                await handler();
            } finally {
                event.completed();
            }
        });

Imagine one ribbon button handler starts a long-running operation that spans 30 seconds. And another button handler just does a very short operation and invokes Office.addin.showAsTaskpane(). The user clicks the first button and then the second one.

In this scenario, the task pane will not be shown until the first handler completes, which is very poor UX: the user expects to see the task pane almost instantly.

We could restructure our code like this to work around the bug:

        Office.actions.associate(name, async event => {
            event.completed();

            await handler();
        });

But we don't want this because we lose the status bar indication: "Velixo is working on your XXX", and it's also very poor UX as the user doesn't know that something is happening in the add-in.

ghost commented 1 year ago

Thank you for letting us know about this issue. We will take a look shortly. Thanks.

pkkj commented 1 year ago

Hi wh1t3cAt1k,

One more question for your scenario. Does your add-in use shared-runtime for this ribbon action?

Thanks,

wh1t3cAt1k commented 1 year ago

Hi @pkkj, yes it uses the shared runtime... I thought it was the most common choice nowadays so didn't mention it explicitly.

madhavagrawal17 commented 1 year ago

@wh1t3cAt1k, This is expected behavior as we execute each ui-less action one by one in a queue. We only execute the next action once the event.completed() is called on the previous action. That is by design. If you want to have a button to Show the Taskpane immediately then can you use the ShowTaskpane action in the manifest.

wh1t3cAt1k commented 1 year ago

@madhavagrawal17 in our case it's not really "UI-less" as we display the operation status, progress, and errors in the task pane under some circumstances.

We are using a shared runtime add-in and contextual tabs. To the best of my knowledge, the handlers for contextual tab buttons always accept an event object and need to complete it.

Inside those handlers, we may dynamically decide, at an arbitrary moment, to show the task pane for example, to display an error details prompt. If an error does not happen, we do not show it.

In addition to that, while an operation started by one button is running, we also need the user to always be able to open the task pane immediately using other contextual tab buttons.

To clarify, those other buttons open the task pane and then take the user to the correct screen state. (e.g. to "Operation status" React component). So, using just the manifest button for that does not achieve the above goal. Furthermore, it is confusing — with the default manifest action, the user has to navigate away from our ribbon to a completely different (Home) Excel ribbon tab.

Currently, we can achieve most of our goals if we immediately call event.complete() inside the handler. But in this case, we lose the status bar indication, which we want to keep.

Therefore, I would argue that the sequential execution design does not lend itself well to modern shared runtime add-ins, and there needs to be some sort of support for concurrent handlers.

wh1t3cAt1k commented 1 year ago

We got an update about https://github.com/OfficeDev/office-js/issues/2877 (but we don't know the exact fix version unfortunately).

Does it impact the current issue in any way? @penglongzhaochina @madhavagrawal17 @Wenjun-Gong @pkkj ?

wh1t3cAt1k commented 1 year ago

@LiubovKabir just confirmed on our side that the current issue is not solved. You still cannot run another handler while the previous one is being executed.

The context is this: we have an operation called "Refresh" which takes a very long time to complete, but we still want to display a status bar indication while it's running, so we call event.complete() at the end (in the finally block). This can take seconds or even minutes on large files.

On the other hand, there are shorter operations that can also be invoked from our contextual tab, and for them we want to show the side panel and perform some unrelated stuff immediately, even if the refresh is currently running.

@pkkj @madhavagrawal17 of course I might not have the full picture, but I don't see on my side what bad can happen if you allow handlers to run concurrently?

As long as there is a strict execution queue in Excel.run, we should be fine.