Skip to content

Commit

Permalink
Introduce a --viewer-container-height CSS variable to simplify the …
Browse files Browse the repository at this point in the history
…code

This new CSS variable will allow us to simplify a couple of different viewer components, since we no longer need to use JavaScript-based hacks and can directly set the CSS rules instead. In particular:

 - The `BaseViewer`-handling, used as part of the code that will center pages *vertically* in PresentationMode, can be simplified.
   By using CSS to control the height of the "dummy"-page we avoid unnecessarily invalidating the scale-value, which can reduce *some* unneeded re-rendering while PresentationMode is active.

 - The `SecondaryToolbar.#setMaxHeight`-method, and its associated parameters, are no longer necessary and can be completely removed.

Note that in order for things to work correctly in general, the new `--viewer-container-height` CSS variable must potentially be updated on any window-based "resize"-event (even when there's no zooming). While this is currently only done in the default viewer, that shouldn't be an issue since neither PresentationMode nor Toolbar-functionality is included in the "viewer components".
  • Loading branch information
Snuffleupagus committed May 6, 2022
1 parent cfac6fa commit 9817230
Show file tree
Hide file tree
Showing 6 changed files with 21 additions and 51 deletions.
3 changes: 2 additions & 1 deletion web/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -574,7 +574,6 @@ const PDFViewerApplication = {

this.secondaryToolbar = new SecondaryToolbar(
appConfig.secondaryToolbar,
container,
eventBus
);

Expand Down Expand Up @@ -2401,6 +2400,8 @@ function webViewerSpreadModeChanged(evt) {

function webViewerResize() {
const { pdfDocument, pdfViewer } = PDFViewerApplication;
pdfViewer.updateContainerHeightCss();

if (!pdfDocument) {
return;
}
Expand Down
26 changes: 13 additions & 13 deletions web/base_viewer.js
Original file line number Diff line number Diff line change
Expand Up @@ -300,6 +300,7 @@ class BaseViewer {
if (this.removePageBorders) {
this.viewer.classList.add("removePageBorders");
}
this.updateContainerHeightCss();
// Defer the dispatching of this event, to give other viewer components
// time to initialize *and* register 'baseviewerinit' event listeners.
Promise.resolve().then(() => {
Expand Down Expand Up @@ -936,7 +937,6 @@ class BaseViewer {
if (this.isInPresentationMode) {
const dummyPage = document.createElement("div");
dummyPage.className = "dummyPage";
dummyPage.style.height = `${this.container.clientHeight}px`;
spread.appendChild(dummyPage);
}

Expand Down Expand Up @@ -994,14 +994,6 @@ class BaseViewer {
* only because of limited numerical precision.
*/
#isSameScale(newScale) {
if (
this.isInPresentationMode &&
this.container.clientHeight !== this.#previousContainerHeight
) {
// Ensure that the current page remains centered vertically if/when
// the window is resized while PresentationMode is active.
return false;
}
return (
newScale === this._currentScale ||
Math.abs(newScale - this._currentScale) < 1e-15
Expand Down Expand Up @@ -1062,8 +1054,7 @@ class BaseViewer {
if (this.defaultRenderingQueue) {
this.update();
}

this.#previousContainerHeight = this.container.clientHeight;
this.updateContainerHeightCss();
}

/**
Expand Down Expand Up @@ -1096,8 +1087,7 @@ class BaseViewer {
hPadding = vPadding = 4;
} else if (this.removePageBorders) {
hPadding = vPadding = 0;
}
if (this._scrollMode === ScrollMode.HORIZONTAL) {
} else if (this._scrollMode === ScrollMode.HORIZONTAL) {
[hPadding, vPadding] = [vPadding, hPadding]; // Swap the padding values.
}
const pageWidthScale =
Expand Down Expand Up @@ -2077,6 +2067,16 @@ class BaseViewer {
} while (--steps > 0 && newScale > MIN_SCALE);
this.currentScaleValue = newScale;
}

updateContainerHeightCss() {
const height = this.container.clientHeight;

if (height !== this.#previousContainerHeight) {
this.#previousContainerHeight = height;

this._doc.style.setProperty("--viewer-container-height", `${height}px`);
}
}
}

export { BaseViewer, PagesCountLimit, PDFPageViewBuffer };
3 changes: 2 additions & 1 deletion web/pdf_viewer.css
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
@import url(xfa_layer_builder.css);

:root {
--viewer-container-height: 0;
--pdfViewer-padding-bottom: 0;
--page-margin: 1px auto -8px;
--page-border: 9px solid transparent;
Expand Down Expand Up @@ -57,7 +58,7 @@
.pdfViewer .dummyPage {
position: relative;
width: 0;
/* The height is set via JS, see `BaseViewer.#ensurePageViewVisible`. */
height: var(--viewer-container-height);
}

.pdfViewer.removePageBorders .page {
Expand Down
34 changes: 2 additions & 32 deletions web/secondary_toolbar.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
* limitations under the License.
*/

import { SCROLLBAR_PADDING, ScrollMode, SpreadMode } from "./ui_utils.js";
import { ScrollMode, SpreadMode } from "./ui_utils.js";
import { CursorTool } from "./pdf_cursor_tools.js";
import { PagesCountLimit } from "./base_viewer.js";

Expand All @@ -22,9 +22,6 @@ import { PagesCountLimit } from "./base_viewer.js";
* @property {HTMLDivElement} toolbar - Container for the secondary toolbar.
* @property {HTMLButtonElement} toggleButton - Button to toggle the visibility
* of the secondary toolbar.
* @property {HTMLDivElement} toolbarButtonContainer - Container where all the
* toolbar buttons are placed. The maximum height of the toolbar is controlled
* dynamically by adjusting the 'max-height' CSS property of this DOM element.
* @property {HTMLButtonElement} presentationModeButton - Button for entering
* presentation mode.
* @property {HTMLButtonElement} openFileButton - Button to open a file.
Expand Down Expand Up @@ -52,13 +49,11 @@ import { PagesCountLimit } from "./base_viewer.js";
class SecondaryToolbar {
/**
* @param {SecondaryToolbarOptions} options
* @param {HTMLDivElement} mainContainer
* @param {EventBus} eventBus
*/
constructor(options, mainContainer, eventBus) {
constructor(options, eventBus) {
this.toolbar = options.toolbar;
this.toggleButton = options.toggleButton;
this.toolbarButtonContainer = options.toolbarButtonContainer;
this.buttons = [
{
element: options.presentationModeButton,
Expand Down Expand Up @@ -154,12 +149,8 @@ class SecondaryToolbar {
pageRotateCcw: options.pageRotateCcwButton,
};

this.mainContainer = mainContainer;
this.eventBus = eventBus;

this.opened = false;
this.containerHeight = null;
this.previousContainerHeight = null;

this.reset();

Expand All @@ -169,9 +160,6 @@ class SecondaryToolbar {
this.#bindCursorToolsListener(options);
this.#bindScrollModeListener(options);
this.#bindSpreadModeListener(options);

// Bind the event listener for adjusting the 'max-height' of the toolbar.
this.eventBus._on("resize", this.#setMaxHeight.bind(this));
}

/**
Expand Down Expand Up @@ -322,8 +310,6 @@ class SecondaryToolbar {
return;
}
this.opened = true;
this.#setMaxHeight();

this.toggleButton.classList.add("toggled");
this.toggleButton.setAttribute("aria-expanded", "true");
this.toolbar.classList.remove("hidden");
Expand All @@ -346,22 +332,6 @@ class SecondaryToolbar {
this.open();
}
}

#setMaxHeight() {
if (!this.opened) {
return; // Only adjust the 'max-height' if the toolbar is visible.
}
this.containerHeight = this.mainContainer.clientHeight;

if (this.containerHeight === this.previousContainerHeight) {
return;
}
this.toolbarButtonContainer.style.maxHeight = `${
this.containerHeight - SCROLLBAR_PADDING
}px`;

this.previousContainerHeight = this.containerHeight;
}
}

export { SecondaryToolbar };
3 changes: 2 additions & 1 deletion web/viewer.css
Original file line number Diff line number Diff line change
Expand Up @@ -482,7 +482,8 @@ select {

#secondaryToolbarButtonContainer {
max-width: 220px;
max-height: 400px;
min-height: 26px;
max-height: calc(var(--viewer-container-height) - 40px);
overflow-y: auto;
margin-bottom: -4px;
}
Expand Down
3 changes: 0 additions & 3 deletions web/viewer.js
Original file line number Diff line number Diff line change
Expand Up @@ -100,9 +100,6 @@ function getViewerConfiguration() {
secondaryToolbar: {
toolbar: document.getElementById("secondaryToolbar"),
toggleButton: document.getElementById("secondaryToolbarToggle"),
toolbarButtonContainer: document.getElementById(
"secondaryToolbarButtonContainer"
),
presentationModeButton: document.getElementById(
"secondaryPresentationMode"
),
Expand Down

0 comments on commit 9817230

Please sign in to comment.