[24.2] Activity bar add new workflow button

[ElectronicBlueberry]

fixes #19274

not technically a bug, but since this is a big UX gain for a relatively small change, I would like if we can still get it into this release. Otherwise re-targeting to dev is fine.

How to test the changes?

(Select all options that apply)

• I've included appropriate automated tests.
• This is a refactoring of components with existing test coverage.
• Instructions for manual testing are as follows:
  1. [add testing steps and prerequisites here if you didn't write automated tests covering all your changes]

License

• I agree to license these and all my past contributions to the core galaxy codebase under the MIT license.

[jmchilton]

I think given that it is a new feature we're highlighting it in the release notes and we want our researchers to only need to learn it once - I think it is a good addition for 24.2. The Selenium tests seem legitimate - I think the more robust input adder in #19216 would resolve it but they are going to conflict with each other. If I have some time tonight I'll try to break a commit out of there for this.

[mvdbeek]

Love that you can create a new one from there, but shouldn't this be together with save ? (in fact save, run, download, undo and redo seem like they should all be grouped ?)

[ElectronicBlueberry]

Do you have an idea on how to group them? I'm generally for grouping the single action activities, I just havent found a way that makes a lot of sense. Regarding the changes activity, I'd rather not group this, since it has it's own panel. It was also too hidden before, and this seems like it would be hiding it more again.

[mvdbeek]

I was thinking that we could place them where the redo undo buttons are ? I'll open a separate issue for that, I don't think that needs to hold up this PR.

[ElectronicBlueberry]

I've just looked into the test failure and it's the same vueuse error stat pops up randomly with the persistent state ref not updating in some strangely specific circumstances, which I've struggled with before. Fixing this is a rabbit hole which ends at webpack packaging too much code into web workers, which keeps us from directly accessing the local storage api, without going through libraries. Since I'm guessing were not making the switch to vite any time soon, I see the following options:

• Publish our own persistent ref into a package
• Put a work around in the selenium tests, which clicks links twice if the click has no effect the first time
• Dig around in webpack as to why it's over-bundling code. I've tried this before to no avail, but maybe someone has a different idea that may work
• Delay this feature until we upgrade to vite... again :/

Open to any other ideas

[dannon]

We definitely aren't switching to vite for 24.2, but having updated the branch again recently I'd love to try to get it into 25.0. That said, these workers were problematic for me there as well -- I'll refresh my memory as to what was going on there tomorrow.

We discussed this in a recent meeting and ended up simply bypassing using a worker for jest tests, and I'd be okay with us doing the same for this. The broader conversation was about whether it was worth having a worker at all -- I wish we had data on how frequently the expensive clientside search actually kicked in before making that call though.

I'll spend some time on this in the morning - maybe the problems I was running into with the worker in the vite branch shed light on why webpack is bundling it poorly.

[ElectronicBlueberry]

We discussed this in a recent meeting and ended up simply bypassing using a worker for jest tests, and I'd be okay with us doing the same for this

The Problem unfortunately is not jest related. The production build actually fails to run workers all together.

The Problem stems from here: https://github.com/vueuse/vueuse/blob/main/packages/core/useStorage/index.ts

Under some specific states, which I haven't found a way to predict, this composable fails to update the ref when it is first written to.
I worked around this by writing a custom composable, which focuses only on local storage, and is therefore much simpler and more predictable than the vueuse one.

This works fantastically in dev builds, but on production builds all workers fail to start, due to some code accessing the local storage being loaded into all workers, and the local storage api is not available in workers, crashing the workers.

Libraries are bundled separately from our main code, and do not have this issue.

Writing all of this down I think the best way to move forward is to publish the custom composable in it's own library. This (hopefully) actually fixes the issue, rather than working around it for tests to pass, and we do not have to spend too much time messing with the webpack configuration.

[dannon]

@ElectronicBlueberry I am guessing you saw #19316 based on the last commits just added here; that's one of the things I ran into trying to walk through all this today. Let's get that in first independent of the core changes here?

[ElectronicBlueberry]

@dannon I did not. I had these commits locally for a bit before pushing them here, to make sure they work properly. I'll have a look at your PR

edit: ah, we found the same thing :D
Yes, it's unrelated. I just found it when stepping through the code an fixed it on the side. We can keep this as a separate PR, if you want.

[dannon]

Hah, yeah, I found it stepping through this PR trying to fully understand the ref issue ;)

Happy to cherry pick your second commit to that branch and get it in separately, just go keep the issues untangled?

[ElectronicBlueberry]

Finally managed to fully separate the worker bundle from the other bundles. This allowed me to create a custom local storage binding, which uncovered a slew of other bugs that were shadowed by the old buggy local storage ref. This PR now also fixes some clicks not registering randomly in the activity bar, and enables moving forward with some old PRs 🥳

Also significantly simplifies the useUserLocalStorage composable we use in many places, as it now longer needs to be built encapsulating another composable which wasn't built with async storage access in mind.