Skip to content

Commit

Permalink
Enable PDF side-by-side mode for all
Browse files Browse the repository at this point in the history
Remove check for experimental config option—always lay out PDF
side-by-side with sidebar when there is enough room to do so.

Fixes hypothesis/product-backlog#1139
  • Loading branch information
lyzadanger committed Sep 28, 2020
1 parent 2f6ec3b commit 617ed97
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 87 deletions.
18 changes: 3 additions & 15 deletions src/annotator/pdf-sidebar.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,23 +32,11 @@ export default class PdfSidebar extends Sidebar {
this.pdfViewer = this.window.PDFViewerApplication?.pdfViewer;
this.pdfContainer = this.window.PDFViewerApplication?.appConfig?.appContainer;

// Prefer to lay out the sidebar and the PDF side-by-side (with the sidebar
// not overlapping the PDF) when space allows
this.sideBySide = !!(
config.experimental?.pdfSideBySide &&
this.pdfViewer &&
this.pdfContainer
);

// Is the current state of the layout side-by-side?
// Is the PDF currently displayed side-by-side with the sidebar?
this.sideBySideActive = false;

if (this.sideBySide) {
this.subscribe('sidebarLayoutChanged', state =>
this.fitSideBySide(state)
);
this.window.addEventListener('resize', () => this.fitSideBySide());
}
this.subscribe('sidebarLayoutChanged', state => this.fitSideBySide(state));
this.window.addEventListener('resize', () => this.fitSideBySide());
}

/**
Expand Down
79 changes: 7 additions & 72 deletions src/annotator/test/pdf-sidebar-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,23 +66,10 @@ describe('PdfSidebar', () => {
});

context('side-by-side mode configured', () => {
it('enables side-by-side mode if config and PDF js are present', () => {
const sidebar = createPdfSidebar({
experimental: {
pdfSideBySide: true,
},
});
assert.isTrue(sidebar.sideBySide);
});

describe('when window is resized', () => {
it('attempts to lay out side-by-side', () => {
sandbox.stub(window, 'innerWidth').value(1300);
const sidebar = createPdfSidebar({
experimental: {
pdfSideBySide: true,
},
});
const sidebar = createPdfSidebar();

window.dispatchEvent(new Event('resize'));

Expand All @@ -97,11 +84,7 @@ describe('PdfSidebar', () => {

it('resizes and activates side-by-side mode', () => {
sandbox.stub(window, 'innerWidth').value(1300);
const sidebar = createPdfSidebar({
experimental: {
pdfSideBySide: true,
},
});
const sidebar = createPdfSidebar();
sidebar._lastSidebarLayoutState = {
expanded: true,
width: 428,
Expand All @@ -119,11 +102,7 @@ describe('PdfSidebar', () => {

it('does not activate side-by-side mode if there is not enough room', () => {
sandbox.stub(window, 'innerWidth').value(800);
const sidebar = createPdfSidebar({
experimental: {
pdfSideBySide: true,
},
});
const sidebar = createPdfSidebar();
sidebar._lastSidebarLayoutState = {
expanded: true,
width: 428,
Expand All @@ -143,11 +122,7 @@ describe('PdfSidebar', () => {
describe('when sidebar layout state changes', () => {
it('resizes and activates side-by-side mode when sidebar expanded', () => {
sandbox.stub(window, 'innerWidth').value(1350);
const sidebar = createPdfSidebar({
experimental: {
pdfSideBySide: true,
},
});
const sidebar = createPdfSidebar();

sidebar.publish('sidebarLayoutChanged', [
{ expanded: true, width: 428, height: 728 },
Expand All @@ -169,11 +144,7 @@ describe('PdfSidebar', () => {
it('activates side-by-side mode for each relative zoom mode', () => {
fakePDFViewerApplication.pdfViewer.currentScaleValue = zoomMode;
sandbox.stub(window, 'innerWidth').value(1350);
const sidebar = createPdfSidebar({
experimental: {
pdfSideBySide: true,
},
});
const sidebar = createPdfSidebar();

sidebar.publish('sidebarLayoutChanged', [
{ expanded: true, width: 428, height: 728 },
Expand All @@ -187,11 +158,7 @@ describe('PdfSidebar', () => {

it('deactivates side-by-side mode when sidebar collapsed', () => {
sandbox.stub(window, 'innerWidth').value(1350);
const sidebar = createPdfSidebar({
experimental: {
pdfSideBySide: true,
},
});
const sidebar = createPdfSidebar();

sidebar.publish('sidebarLayoutChanged', [
{ expanded: false, width: 428, height: 728 },
Expand All @@ -203,11 +170,7 @@ describe('PdfSidebar', () => {

it('does not activate side-by-side mode if there is not enough room', () => {
sandbox.stub(window, 'innerWidth').value(800);
const sidebar = createPdfSidebar({
experimental: {
pdfSideBySide: true,
},
});
const sidebar = createPdfSidebar();

sidebar.publish('sidebarLayoutChanged', [
{ expanded: true, width: 428, height: 728 },
Expand All @@ -219,32 +182,4 @@ describe('PdfSidebar', () => {
});
});
});

context('side-by-side mode not configured', () => {
it('does not enable side-by-side mode', () => {
const sidebar = createPdfSidebar({});

assert.isFalse(sidebar.sideBySide);
});

it('does not attempt to resize PDF container on window resize', () => {
const sidebar = createPdfSidebar({});

window.dispatchEvent(new Event('resize'));

assert.isFalse(sidebar.sideBySideActive);
assert.notCalled(fakePDFViewerUpdate);
});

it('does not attempt to resize PDF container on sidebar layout change', () => {
const sidebar = createPdfSidebar({});

sidebar.publish('sidebarLayoutChanged', [
{ expanded: true, width: 428, height: 728 },
]);

assert.isFalse(sidebar.sideBySideActive);
assert.notCalled(fakePDFViewerUpdate);
});
});
});

0 comments on commit 617ed97

Please sign in to comment.