The latest updates on your projects. Learn more about Vercel for Git ↗︎

Thanks @demiankatz , I've done those bits of clean-up.

I've also changed it so that it only auto-closes in 'sm' size for now. I agree that ultimately we want it to auto-close if its initial state is closed. I tried something that should work, but it doesn't work in this branch yet as I think determining the open/closed state on initiation might still be up in the air for mobile. Here's the method I used (partly for my own reference if we implement it later):

• leftPanel.ts currently creates a new boolean 'shouldOpenPanel' on inititiation:

` init(): void {
super.init();

const shouldOpenPanel: boolean = Bools.getBool(
  this.extension.getSettings().leftPanelOpen,
  this.options.panelOpen
);

if (shouldOpenPanel) {
  this.toggle(true);
}

}`

• move shouldOpenPanel out of the init function so that its accessible as a property on leftPanel;
• Check for this in the event subscriptions in contentLeftPanel like so:
  this.extensionHost.subscribe(IIIFEvents.TREE_NODE_SELECTED, () => { if (this.shouldOpenPanel) { this.toggle(true) } })

It would be useful if we can talk about the initial open/closed state with @LlGC-jop before implementing this.

Getting the initial metric and making it a property of the baseExpandPanel in order to determine autoclosing is proving a bit tricky. The _updateMetric() method on BaseExtension supplies the current metric on init, but for some reason it's got a 1ms delay on it, so it's happening after the baseExpandPanels are initiating, so checking for 'sm' at higher class levels is giving 'undefined'. Will return to this tomorrow.

if checking the metric value from the panels worked, then putting this either on leftPanel or contentLeftPanel would cover the conditions for setting the initial state, taking into account the last user state of the panel, the config on init, and the size on init:

this.shouldOpenPanel = Bools.getBool( this.extension.getSettings().leftPanelOpen, this.options.panelOpen && this.extension.metric != 'sm', )

if (this.shouldOpenPanel) { this.toggle(true); }

(this is probably why the 'isMobile()' on the baseExtension exists, as it checks this.metric there rather than checking this.extension.metric.)

@jamesmisson, I wonder if this is a situation where emitting an event might help... though that gets messy pretty quickly.

Thanks, @jamesmisson -- everything looks good now. I'm not going to approve this yet since I expect we may not want to merge this PR as-is. I'll wait until our conversation with @LlGC-jop tomorrow to figure out which PRs are actually destined for merging to dev and which will be closed as partial work in progress.

Superseded by #1343.