Sakurai/pdf.js
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Only display a notification on the sidebarToggle-button, and not the individual view-buttons (PR 7959 follow-up)

Browse Source 
The whole purpose of showing a notification on the `sidebarToggle` button, when the sidebar is closed, was to give users *some* kind of indication that the PDF document contains outline/attachments/layers without having to manually open the sidebar to check.
However, in the implementation in PR 7959, I also added notifications for each view-buttons in the sidebar. Looking back at this, I've always questioned the value of the last part, since the view-buttons already have a `disabled`-state which shows if they're available or not. Hence we're actually, in a sense, duplicating notifications for the outline/attachments/layers-buttons without adding (in my opinion) all that much overall value.

All-in-all, I'm thus proposing that we only display the notification on the `sidebarToggle`-button itself, since that should really be sufficient here, which also allows us to simplify the relevant code a fair bit.
This commit is contained in:
Jonas Jenwald 2021-01-05 11:37:11 +01:00
parent 172abc02e1
commit e997f60656
1 changed files with 15 additions and 64 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

79
web/pdf_sidebar.js
View File

@ -25,8 +25,6 @@ const UI_NOTIFICATION_CLASS = "pdfSidebarNotification";
* @property {PDFThumbnailViewer} pdfThumbnailViewer - The thumbnail viewer.
* @property {EventBus} eventBus - The application event bus.
* @property {IL10n} l10n - The localization service.
* @property {boolean} [disableNotification] - Disable the notification for
* documents containing outline/attachments. The default value is `false`.
*/
/**
@ -69,7 +67,6 @@ class PDFSidebar {
pdfThumbnailViewer,
eventBus,
l10n = NullL10n,
disableNotification = false,
}) {
this.isOpen = false;
this.active = SidebarView.THUMBS;
@ -103,7 +100,6 @@ class PDFSidebar {
this.eventBus = eventBus;
this.l10n = l10n;
this._disableNotification = disableNotification;
this._addEventListeners();
}
@ -111,7 +107,7 @@ class PDFSidebar {
reset() {
this.isInitialViewSet = false;
this._hideUINotification(null);
this._hideUINotification(/* reset = */ true);
this.switchView(SidebarView.THUMBS);
this.outlineButton.disabled = false;
@ -259,7 +255,6 @@ class PDFSidebar {
if (isViewChanged) {
this._dispatchEvent();
}
this._hideUINotification(this.active);
return isViewChanged;
}
@ -278,7 +273,7 @@ class PDFSidebar {
this._forceRendering();
this._dispatchEvent();
this._hideUINotification(this.active);
this._hideUINotification();
}
close() {
@ -347,11 +342,7 @@ class PDFSidebar {
/**
* @private
*/
_showUINotification(view) {
if (this._disableNotification) {
return;
}
_showUINotification() {
this.l10n
.get(
"toggle_sidebar_notification2.title",
@ -366,66 +357,26 @@ class PDFSidebar {
// Only show the notification on the `toggleButton` if the sidebar is
// currently closed, to avoid unnecessarily bothering the user.
this.toggleButton.classList.add(UI_NOTIFICATION_CLASS);
} else if (view === this.active) {
// If the sidebar is currently open *and* the `view` is visible, do not
// bother the user with a notification on the corresponding button.
return;
}
switch (view) {
case SidebarView.OUTLINE:
this.outlineButton.classList.add(UI_NOTIFICATION_CLASS);
break;
case SidebarView.ATTACHMENTS:
this.attachmentsButton.classList.add(UI_NOTIFICATION_CLASS);
break;
case SidebarView.LAYERS:
this.layersButton.classList.add(UI_NOTIFICATION_CLASS);
break;
}
}
/**
* @private
*/
_hideUINotification(view) {
if (this._disableNotification) {
return;
_hideUINotification(reset = false) {
if (this.isOpen || reset) {
// Only hide the notification on the `toggleButton` if the sidebar is
// currently open, or when the current PDF document is being closed.
this.toggleButton.classList.remove(UI_NOTIFICATION_CLASS);
}
const removeNotification = sidebarView => {
switch (sidebarView) {
case SidebarView.OUTLINE:
this.outlineButton.classList.remove(UI_NOTIFICATION_CLASS);
break;
case SidebarView.ATTACHMENTS:
this.attachmentsButton.classList.remove(UI_NOTIFICATION_CLASS);
break;
case SidebarView.LAYERS:
this.layersButton.classList.remove(UI_NOTIFICATION_CLASS);
break;
}
};
if (!this.isOpen && view !== null) {
// Only hide the notifications when the sidebar is currently open,
// or when it is being reset (i.e. `view === null`).
return;
if (reset) {
this.l10n
.get("toggle_sidebar.title", null, "Toggle Sidebar")
.then(msg => {
this.toggleButton.title = msg;
});
}
this.toggleButton.classList.remove(UI_NOTIFICATION_CLASS);
if (view !== null) {
removeNotification(view);
return;
}
// Remove all sidebar notifications on reset.
for (view in SidebarView) {
removeNotification(SidebarView[view]);
}
this.l10n.get("toggle_sidebar.title", null, "Toggle Sidebar").then(msg => {
this.toggleButton.title = msg;
});
}
/**
@ -475,7 +426,7 @@ class PDFSidebar {
button.disabled = !count;
if (count) {
this._showUINotification(view);
this._showUINotification();
} else if (this.active === view) {
// If the `view` was opened by the user during document load,
// switch away from it if it turns out to be empty.