Allow pluginTarget to be passed as a prop to WindowTopBarPluginMenu

[marlo-longley]

It seems that after this work to refactor some of the plugin system, there are some impacts for downstream.

Locally we are upgrading to Mirador 4 and ran into an issue with our custom plugin, which reuses the WindowTopBarPluginMenu component from Mirador.

After the Mirador plugin changes linked above, our local plugin no longer works, I think due to this line changed upstream: const { PluginComponents } = usePlugins('WindowTopBarPluginMenu'); , which hardcodes the plugin name.

Open to thoughts, but changing this and then allowing the downstream app to this worked well:

const CustomMenuComponent = (props) => {
  return (
    <WindowTopBarPluginMenu
        menuIcon={<CustomIcon />}
        pluginTarget="CustomMenuComponent"
        windowId={props.windowId}
    />
  );
};

I would love to get this into Mirador 4 or learn of another solution, otherwise our local plugin doesn't work.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.19%. Comparing base (feb1375) to head (787ffe6).
⚠️ Report is 6 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4165      +/-   ##
==========================================
+ Coverage   94.98%   95.19%   +0.20%     
==========================================
  Files         325      325              
  Lines       16153    16154       +1     
  Branches     2535     2543       +8     
==========================================
+ Hits        15343    15377      +34     
+ Misses        805      772      -33     
  Partials        5        5              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[marlo-longley]

I am addressing a question I got from @lutzhelm about this PR:

Do we need the same behavior for other plugin targets? Or is this a special case?

I looked into this and believe it is a special case. Here's the background. This pattern of hardcoding the component name as targetName was introduced in this PR.

My team at Stanford ran into an issue because we are creating a new instance of WindowTopBarPluginMenu as a plugin / custom component. This means that two different components, internally, were trying to make use of the same targetName. The use case is adding an additional custom menu to the top bar (in addition to the default one shown below). I heard from colleagues at another university that they rely on a similar pattern and ran into the same issue with Mirador 4.

So anecdotally, adding an extra, custom WindowTopBarPluginMenu seems to be a pattern that people are using.

The other components that hardcode targetName are:
AttributionPanel, BackgroundPluginArea, CanvasInfo, ErrorContent , ManifestInfo, ManifestRelatedLinks, OpenSeadragonViewer, Window, WindowAuthenticationBar, WindowCanvasNavigationControls, WindowTopBarPluginArea, WindowTopMenu, WorkspaceAdd, WorkspaceControlPanelButtons, WorkspaceMenu , WorkspaceOptionsMenu.

I don't know if anyone is creating duplicate/custom instances of these components in their custom apps and whether we need to support that. I would favor just making the change in WindowTopBarPluginMenu for now.

[jcoyne]

This looks good to me. Let's see if anyone else has comments, or I'll merge it early next week.