Commit e15e597b authored by Robert Knight's avatar Robert Knight

Fix sidebar container initialization when "clean" theme is used

When the "clean" theme is used, the sidebar's toggle button is not created and
constructing the `DragHandler` failed because it was passed a `null` target.
Update the types in `ToolbarController.sidebarToggleButton` to reflect that the
value can be null and handle this in `Sidebar` by not initializing
`_dragResizeHandler` in this case.
parent 39cf23df
...@@ -105,7 +105,7 @@ type DragResizeState = { ...@@ -105,7 +105,7 @@ type DragResizeState = {
export class Sidebar implements Destroyable { export class Sidebar implements Destroyable {
private _emitter: Emitter; private _emitter: Emitter;
private _config: SidebarContainerConfig & SidebarConfig; private _config: SidebarContainerConfig & SidebarConfig;
private _dragResizeHandler: DragHandler; private _dragResizeHandler: DragHandler | undefined;
private _dragResizeState: DragResizeState; private _dragResizeState: DragResizeState;
private _listeners: ListenerCollection; private _listeners: ListenerCollection;
private _layoutState: SidebarLayout; private _layoutState: SidebarLayout;
...@@ -292,10 +292,15 @@ export class Sidebar implements Destroyable { ...@@ -292,10 +292,15 @@ export class Sidebar implements Destroyable {
initial: null, initial: null,
final: null, final: null,
}; };
const toggleButton = this.toolbar.sidebarToggleButton;
if (toggleButton) {
this._dragResizeHandler = new DragHandler({ this._dragResizeHandler = new DragHandler({
target: this.toolbar.sidebarToggleButton, target: toggleButton,
onDrag: event => this._onDragSidebarToggleButton(event), onDrag: event => this._onDragSidebarToggleButton(event),
}); });
}
this.close(); this.close();
// Publisher-provided callback functions // Publisher-provided callback functions
...@@ -327,7 +332,7 @@ export class Sidebar implements Destroyable { ...@@ -327,7 +332,7 @@ export class Sidebar implements Destroyable {
this._sidebarRPC.destroy(); this._sidebarRPC.destroy();
this.bucketBar?.destroy(); this.bucketBar?.destroy();
this._listeners.removeAll(); this._listeners.removeAll();
this._dragResizeHandler.destroy(); this._dragResizeHandler?.destroy();
if (this._hypothesisSidebar) { if (this._hypothesisSidebar) {
// Explicitly unmounting the "messages" element, to make sure effects are clean-up // Explicitly unmounting the "messages" element, to make sure effects are clean-up
render(null, this._messagesElement!); render(null, this._messagesElement!);
......
...@@ -154,15 +154,26 @@ describe('Sidebar', () => { ...@@ -154,15 +154,26 @@ describe('Sidebar', () => {
fakeDragHandler = { fakeDragHandler = {
destroy: sinon.stub(), destroy: sinon.stub(),
}; };
FakeDragHandler = sinon.stub().returns(fakeDragHandler); FakeDragHandler = sinon.stub().callsFake(options => {
assert.isNotNull(options.target);
assert.isFunction(options.onDrag);
return fakeDragHandler;
});
const toggleButton = document.createElement('button');
fakeToolbar = { fakeToolbar = {
getWidth: sinon.stub().returns(100), getWidth: sinon.stub().returns(100),
useMinimalControls: false, useMinimalControls: false,
sidebarOpen: false, sidebarOpen: false,
newAnnotationType: 'note', newAnnotationType: 'note',
highlightsVisible: false, highlightsVisible: false,
sidebarToggleButton: document.createElement('button'), get sidebarToggleButton() {
if (this.useMinimalControls) {
return null;
} else {
return toggleButton;
}
},
}; };
FakeToolbarController = sinon.stub().returns(fakeToolbar); FakeToolbarController = sinon.stub().returns(fakeToolbar);
......
...@@ -27,7 +27,7 @@ export class ToolbarController { ...@@ -27,7 +27,7 @@ export class ToolbarController {
private _toggleSidebar: () => void; private _toggleSidebar: () => void;
private _toggleHighlights: () => void; private _toggleHighlights: () => void;
private _createAnnotation: () => void; private _createAnnotation: () => void;
private _sidebarToggleButton: RefObject<HTMLElement>; private _sidebarToggleButton: RefObject<HTMLButtonElement>;
/** /**
* @param container - Element into which the toolbar is rendered * @param container - Element into which the toolbar is rendered
...@@ -52,7 +52,7 @@ export class ToolbarController { ...@@ -52,7 +52,7 @@ export class ToolbarController {
}; };
/** Reference to the sidebar toggle button. */ /** Reference to the sidebar toggle button. */
this._sidebarToggleButton = createRef<HTMLElement>(); this._sidebarToggleButton = createRef();
this.render(); this.render();
} }
...@@ -115,9 +115,11 @@ export class ToolbarController { ...@@ -115,9 +115,11 @@ export class ToolbarController {
/** /**
* Return the DOM element that toggles the sidebar's visibility. * Return the DOM element that toggles the sidebar's visibility.
*
* This will be `null` if {@link useMinimalControls} is true.
*/ */
get sidebarToggleButton() { get sidebarToggleButton(): HTMLButtonElement | null {
return this._sidebarToggleButton.current as HTMLButtonElement; return this._sidebarToggleButton.current;
} }
render() { render() {
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment