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
659 stars 94 forks source link

onCalculated Event set up via Ribbon command is only firing if the Taskpane has been opened #4542

Open hdwatts opened 1 month ago

hdwatts commented 1 month ago

Provide required information needed to triage your issue

I am trying to dirty some excel ranges and await their calculation. I was hoping to do this by adding an onCalculated event to the relevant worksheets or the workbook's worksheet collection.

I am calling this from a ribbon command, and my addin also has a taskpane via the shared runtime. However, this appears to only work if the taskpane has been opened sometime during the workbook's session. When a user opens a workbook, toggles to our ribbon, and tries to run the command, the event's never get called. I can confirm that my custom functions are getting dirtied and recalculated, however the event is not getting fired.

We are currently getting around this by, at the start of the command, forcing the taskpane open for our users. This is not ideal.

This is also difficult to debug, as the dev build auto opens the taskpane. My code is below:

const awaitCalculation = async (
    outerContext: Excel.RequestContext,
    ranges: Excel.Range[],
) => {
    let dirtyWorksheets: Excel.Worksheet[] = []
    const handlersLeftToRun: Set<string> = new Set()
    let eventHandler: (args: Excel.WorksheetCalculatedEventArgs) => Promise<void>

    const startCalcs = async () => {
        const dirtyRanges: Excel.Range[] = dirtyRanges(ranges)
        for (const range of dirtyRanges) {
            range.worksheet.load('id')
        }
        await outerContext.sync()
        dirtyWorksheets = uniqBy(
            dirtyRanges.map(range => range.worksheet),
            ws => ws.id,
        )
        dirtyWorksheets.forEach(worksheet => {
            handlersLeftToRun.add(worksheet.id)
        })

        dirtyWorksheets.forEach(worksheet => {
            worksheet.onCalculated.add(eventHandler)
        })
        await outerContext.sync()
    }

    await new Promise<void>(resolve => {
        eventHandler = async (args: Excel.WorksheetCalculatedEventArgs) => {
            const { worksheetId } = args
            await outerContext.sync()
            handlersLeftToRun.delete(worksheetId)
            if (handlersLeftToRun.size === 0) {
                resolve()
            }
        }
        debugLog('Starting calculations')
        startCalcs()
    }).catch(e => {
        console.log('Error in promise: ', e)
    })

    dirtyWorksheets.forEach(worksheet => {
        worksheet.onCalculated.remove(eventHandler)
    })
}

Your Environment

Expected behavior

The onCalculated event gets called.

Current behavior

The onCalculated event does not get called.

Steps to reproduce

  1. Set up a shared runtime addin with a taskpane and a ribbon
  2. Add a command to dirty some cells, and await their calculation

Link to live example(s)

N/A

Provide additional details

N/A

Context

N/A

Useful logs

N/A

AlexJerabek commented 1 month ago

Thanks for reporting this @hdwatts.

@penglongzhaochina, could you please investigate?

shanshanzheng-dev commented 1 month ago

Hi @hdwatts We'll be looking into this problem, thanks for reporting! we will report back here if we have a suggestion for you.

hdwatts commented 1 month ago

Thanks @shanshanzheng-dev, happy to help further with whatever you may need!

shanshanzheng-dev commented 1 month ago

Hi @hdwatts If possible, could you please provide us your manifest? Because I use our test add-in, these command seem works for me. Thanks.

hdwatts commented 1 month ago

Sure thing @shanshanzheng-dev

It can be found here: https://excel.chronograph.pe/xconnectjs-manifest.xml

It is somewhat difficult to debug in a non production setting, as the command works as long as the taskpane has been opened. When I add it via developer addins (image below), the taskpane opens automatically leading me to be unable to replicate the issue.

image

On production, when our users have it installed from the store, they can have Excel closed and open a fresh workbook. In this user flow, the taskpane does not auto open and they can access the ribbon and execute the command.

shanshanzheng-dev commented 1 month ago

Hi @hdwatts we'll be looking into this issue. seem there is a work around, when I install this addin from appstore:https://pages.store.office.com/addinsinstallpage.aspx?assetid=WA104381685&rs=en-US&correlationId=5effcff6-853b-84f3-96aa-b476d2ffe7a3 This taskpane will auto open. This is my excel version: image And then could you provide us a repro video cause I can‘t repro this issue from my side? Thanks very much.

hdwatts commented 4 weeks ago

Hi @shanshanzheng-dev

Yes, when you install, the taskpane auto opens. However, once installed from the store, you can close Excel and then re-open a workbook. The taskpane will no longer auto open (which is fine!). The ribbon is still available to the user, and they can trigger a command from the ribbon. In my testing, if that command sets up onCalculated event handlers, they do not fire unless the taskpane was opened prior to setting up the handlers.

Right now in that deployed version of the add-in, we have implemented the workaround that auto-opens the taskpane when the command gets ran, so attempting to use that will not work for you.

Let me know if that is enough information. I can look into deploying a version that does not auto open it for a subset of our internal users, in order to get a video for you.

shanshanzheng-dev commented 4 weeks ago

@hdwatts Thanks for provide us this information. It's good if you can provide a video because I can't repro this issue from my side. Or if possible, could you provide the whole code? Thanks.

hdwatts commented 2 weeks ago

@shanshanzheng-dev See attached videos:

This is the broken behavior. The user opens the Excel model, they trigger an action in the ribbon. That action sets up an onCalculated event handler that should wait for the calculation to finish. However, even though you can see that the calculation triggers, the onCalculated event does not get called.

https://github.com/OfficeDev/office-js/assets/16158417/cbad92fa-9043-47b6-9cde-8a0c940be908

Here is the working behavior. The only difference is I have opened the taskpane prior to triggering the action in the ribbon. It is the same action, and after the calculation triggers the event gets called and the workbook is pushed to our servers. After a slight delay while the workbook uploads the user is presented a confirmation screen.

https://github.com/OfficeDev/office-js/assets/16158417/137aa6d8-091d-4ed2-bfab-bcf57f325c18

We currently get around this by always triggering the taskpane to open when the user triggers the push from the ribbon, however ideally that is not needed.

shanshanzheng-dev commented 2 weeks ago

Hi @hdwatts Thanks for these detail information. And so glad you have a work around. It has been put on our backlog . We'll try to investigate the root cause. We unfortunately have no timelines to share at this point. BTW, we'd like to double confirm with you if other Api works good, or it just about event handler? Thanks.

slabereemsft commented 2 weeks ago

Hi @hdwatts,

If I understand your scenario correctly, you are embedding your event handlers in the shared runtime for a task pane but they aren't being called until the user has explicitly viewed the task pane?

Have you set the runtime lifetime element in the manifest to long?

hdwatts commented 2 weeks ago

@slabereemsft I'm creating and cleaning up the event handlers in the command itself, the event handlers are not called or modified by the task pane.

In the broken state, I can see if I add logs that the onCalculated event is set, and even if I manually trigger a calculation call the event still doesn't fire.

My ideal end goal is to have an awaitable function that pauses execution until a round of calculations have been fired.

I am attempting to do that via code similar to the below:

// Take in ranges that we wish to calculate
const awaitCalculation = async (
    outerContext: Excel.RequestContext,
    ranges: Excel.Range[],
) => {
    // Set up variables to track the calculations status
    let dirtyWorksheets: Excel.Worksheet[] = []
    const handlersLeftToRun: Set<string> = new Set()
    let eventHandler: (args: Excel.WorksheetCalculatedEventArgs) => Promise<void>

    // Function to start the Excel calculations and track worksheets
    // that contain our ranges
    const startCalcs = async () => {
        const dirtyRanges: Excel.Range[] = dirtyRanges(ranges)
        for (const range of dirtyRanges) {
            range.worksheet.load('id')
        }
        await outerContext.sync()
        dirtyWorksheets = uniqBy(
            dirtyRanges.map(range => range.worksheet),
            ws => ws.id,
        )
        // Add the worksheet id to our dirty handlers
        dirtyWorksheets.forEach(worksheet => {
            handlersLeftToRun.add(worksheet.id)
        })

        // Establish the onCalculated event for the worksheet
        dirtyWorksheets.forEach(worksheet => {
            worksheet.onCalculated.add(eventHandler)
        })
        await outerContext.sync()
    }

    // Await this promise, which should resolve once
    // calculations have finished running
    await new Promise<void>(resolve => {
        // Set up event handler, remove the worksheet id
        // from the tracked set. If there are no worksheets left
        // then resolve the promise
        eventHandler = async (args: Excel.WorksheetCalculatedEventArgs) => {
            const { worksheetId } = args
            await outerContext.sync()
            handlersLeftToRun.delete(worksheetId)
            if (handlersLeftToRun.size === 0) {
                resolve()
            }
        }

        // With the event handler set up, start the calculation function
        startCalcs()
    }).catch(e => {
        console.log('Error in promise: ', e)
    })

    // We have resolved the promise, clean up the onCalculated event
    dirtyWorksheets.forEach(worksheet => {
        worksheet.onCalculated.remove(eventHandler)
    })
}