While Firefox originally used transition effects for browser UI toolbar buttons, that was removed years ago in https://bugzilla.mozilla.org/show_bug.cgi?id=1393057
Since the PDF.js viewer toolbar transitions were likely based on the Firefox ones, it seems reasonable that these transition effects are removed from PDF.js as well. Besides removing a bunch of CSS, this also makes the toolbar feel ever so slightly more "snappy" without these delays on mouse interaction.
(In order to make it more feasible to modernize/improve the viewer UI, trying to clean-up/simplify existing rules iteratively seems like the most reasonble way to make any progress here w.r.t. being able to test/review things.)
This changes the dropdown icon from being set using the `background` CSS property, to being set with `::after` which is *similar* to all the other toolbar button icons (which use `::before`).
Also tweaks the dropdown `background-color` on `:hover` slightly, since the other changes made it too light.
This patch contains some *much* needed clean-up of, and improvements to, this old code thus addressing a number of issues:
- Set more reasonable *default* values for the widths, in `web/viewer.css`, since the current ones are actually too small even for the (default) `en-US` locale.
This obviously result in a slightly larger zoom dropdown width for many locales, but the more consistent UI does look good to me.
- Stop setting the `min-width`/`max-width` and just use `width` instead.
- Set an explicit `height` of the zoom dropdown, in an attempt to get Google Chrome to display it with the same height as the toolbar buttons.
- Remove additional `Element.setAttribute("style", ...)` usage.
- Actually check *all* of the predefined l10n strings, since the old implementation (implicitly) assumed that the currently selected one was the longest (note e.g. the `ja-JP` locale where one string is considerably longer than the rest).
- Stop invalidating the DOM multiple times when doing the measurements. This was achieved by using a temporary in-memory `canvas`, and we now only need to query the DOM once in order to get the current font properties of the zoom dropdown.
This should hopefully be useful in environments where restrictive CSPs are in effect.
In most cases the replacement is entirely straighforward, and there's only a couple of special cases:
- For the `src/display/font_loader.js` and `web/pdf_outline_viewer.js `cases, since the elements aren't appended to the document yet, it shouldn't matter if the style properties are set one-by-one rather than all at once.
- For the `web/debugger.js` case, there's really no need to set the `padding` inline at all and the definition was simply moved to `web/viewer.css` instead.
*Please note:* There's still *a single* case left, in `web/toolbar.js` for setting the width of the zoom dropdown, which is left intact for now.
The reasons are that this particular case shouldn't matter for users of the general PDF.js library, and that it'd make a lot more sense to just try and re-factor that very old code anyway (thus fixing the `setAttribute` usage in the process).
Having thought *briefly* about using `css-vars-ponyfill`, I'm no longer convinced that it'd be a good idea. The reason is that if we actually want to properly support CSS variables, then that functionality should be available in *all* of our CSS files.
Note in particular the `pdf_viewer.css` file that's built as part of the `COMPONENTS` target, in which case I really cannot see how a rewrite-at-the-client solution would ever be guaranteed to always work correctly and without accidentally touching other CSS in the surrounding application.
All-in-all, simply re-writing the CSS variables at build-time seems much easier and is thus the approach taken in this patch; courtesy of https://github.com/MadLittleMods/postcss-css-variables
By using its `preserve` option, the built files will thus include *both* a fallback and a modern `var(...)` format[1]. As a proof-of-concept this patch removes a couple of manually added fallback values, and converts an additional sidebar related property to use a CSS variable.
---
[1] Comparing the `master` branch with this patch, when using `gulp generic`, produces the following diff for the built `web/viewer.css` file:
```diff
@@ -408,6 +408,7 @@
:root {
--sidebar-width: 200px;
+ --sidebar-transition-duration: 200ms;
}
* {
@@ -550,27 +551,28 @@
position: absolute;
top: 32px;
bottom: 0;
- width: 200px; /* Here, and elsewhere below, keep the constant value for compatibility
- with older browsers that lack support for CSS variables. */
+ width: 200px;
width: var(--sidebar-width);
visibility: hidden;
z-index: 100;
border-top: 1px solid rgba(51, 51, 51, 1);
-webkit-transition-duration: 200ms;
transition-duration: 200ms;
+ -webkit-transition-duration: var(--sidebar-transition-duration);
+ transition-duration: var(--sidebar-transition-duration);
-webkit-transition-timing-function: ease;
transition-timing-function: ease;
}
html[dir='ltr'] #sidebarContainer {
-webkit-transition-property: left;
transition-property: left;
- left: -200px;
+ left: calc(-1 * 200px);
left: calc(-1 * var(--sidebar-width));
}
html[dir='rtl'] #sidebarContainer {
-webkit-transition-property: right;
transition-property: right;
- right: -200px;
+ right: calc(-1 * 200px);
right: calc(-1 * var(--sidebar-width));
}
@@ -640,6 +642,8 @@
#viewerContainer:not(.pdfPresentationMode) {
-webkit-transition-duration: 200ms;
transition-duration: 200ms;
+ -webkit-transition-duration: var(--sidebar-transition-duration);
+ transition-duration: var(--sidebar-transition-duration);
-webkit-transition-timing-function: ease;
transition-timing-function: ease;
}
```
This is not only useful to have one format for consistency, but also to
be able to quickly search for colors for e.g., finding duplicates or
when tweaking the CSS for custom deployments.
If the current viewer is a `PDFSinglePageViewer` instance the Scroll/Spread modes are no-ops, hence displaying buttons that do *nothing* when clicked will probably do very little besides confuse users.
*The danger of fixing one bug is that it can, sometimes too easily, cause another one in the process; sorry for not catching this when testing PR 9816 locally.*
When the *entire* viewer becomes narrow enough, as controlled by the `@media all and (max-width: 840px)` media query, the sidebar should (semi-transparently) overlay the `viewerContainer` instead of moving it horizontally. Unfortunately the changes made in PR 9816 caused the relevant CSS rules to be skipped, because of how the inheritance model works in CSS.
I'm well aware that `!important` is usually advised against, since it "breaks" the CSS inheritance model. However in this case it seemed reasonable to use it, to not only fix the bug at hand but to also prevent similar bugs from occurring in the future.
This is a regression from PR 8993; it causes the pages to be offset horizontally in Presentation Mode, if and only if the sidebar is currently open when the user triggers Presentation Mode.
Please note that while this doesn't seem to affect Firefox, both Chrome and IE are however affected.
Interestingly enough, despite the Chrome extension being affected as well, I cannot find any issue filed about this. (Either Presentation Mode isn't used much at all, or users don't open the sidebar before entering Presentation Mode.)
This builds on the scrolling mode work to add three buttons for joining
page spreads together: one for the default view, with no page spreads,
and two for spreads starting on odd-numbered or even-numbered pages.
By making use of modern CSS features, in this case [CSS variables](https://developer.mozilla.org/en-US/docs/Web/CSS/Using_CSS_variables), implementing sidebar resizing is actually quite simple. Not only will the amount of added code be fairly small, but it should also be easy to maintain since there's no need for complicated JavaScript hacks in order to update the CSS. Another benefit is that the JavaScript code doesn't need to make detailed assumptions about the exact structure of the HTML/CSS code.
Obviously this will not work in older browsers, such as IE, that lack support for CSS variables. In those cases sidebar resizing is simply disabled (via feature detection), and the resizing DOM element hidden, and the behaviour is thus *identical* to the current (fixed-width) sidebar.
However, considering the simplicity of the implementation, I really don't see why limiting this feature to "modern" browsers is a problem.
Finally, note that a few edge-cases meant that the patch is a bit larger than what the basic functionality would dictate. Among those is first of all proper RTL support, and secondly (automatic) resizing of the sidebar when the width of the *entire* viewer changes. Another, pre-existing, issue fixed here is the incomplete interface of `NullL10n`.
*Please note:* This patch has been successfully tested in both LTR and RTL viewer locales, in recent versions of Firefox and Chrome.
Fixes 2072.
If we want to (eventually) make it possible to resize the sidebar, then having its width indirectly affect the toolbar is going to wreck havoc on the media queries used to show/hide buttons in the main toolbar (since many of them depend on the toolbar state, and thus its width).
Updating all of the media queries dynamically with JavaScript seems like a non-starter, given that it'd cause *very* messy code. It thus seem to me that we'd need to fix the position of the sidebar, to have any hope of (in the short term) addressing issue 2072.
Hence, I'm suggesting that the we always layout the sidebar in a consistent vertical position, and only animate the `viewerContainer` rather than the entire `mainContainer`.
Fixes 4052.
Fixes bug 850591.
With the current way that the `HandTool` is implemented, if someone would try to also add a Zoom tool (as issue 1260 asks for) that probably wouldn't work very well given that you'd then have two cursor tools which may not play nice together.
Hence this patch, which attempts to refactor things so that it should be simpler to add e.g. a Zoom tool as well (given that that issue is marked as "good-beginner-bug", and I'm not sure if that really applies considering the current state of the code).
Note that I personally have no interest in implementing a Zoom tool (similar to Adobe Reader) since I wouldn't use it, but I figured that it can't hurt to make this code a bit more future proof.
I noticed that we have a `.toolbarButton.group` CSS class, which is currently applied to some buttons in the viewer. Since it attempts to adjust the `margin-right` property, I was initially a bit puzzled as to why there wasn't different rules for LTR/RTL locales.
However, checking the viewer with the DevTools inspector, in both LTR and RTL locales, I quickly found that the rule in question is *always* being overridden by other CSS rules.
It thus seem to me that while this rule was probably useful at some point, it has been dead for years and could now be removed.
The find functionality is currently not available for small devices
because the find dialog is not responsive. This patch fixes that.
To achieve this goal, the HTML is changed to always show the find
button. To prevent issues because of the addition of an extra button for
small views, the previous/next page buttons are hidden if the view
becomes small. These buttons are not useful anyway because on small
devices navigation is usually done via scrolling. The find functionality
is much more useful to have in this case. Moreover, we wrap the existing
elements into separate `div`s so that the browser can position the
elements itself when the view becomes smaller and logically connected
elements stay together when this happens.
In the CSS, extra rules for the find bar have been added to ensure that
the dialog's doorhanger is always below the find button. All findbar
`div`s are forced to be 32 pixels high to prevent the find message text
being aligned under the checkboxes. Finally, the find message is only
visible when there is actually text to display. This prevents wrapping
issues because, by default, the label has padding and margin even if
there is no text.
A longstanding issue with the viewer is that you cannot tell if a PDF document includes an outline and/or attachments without actually opening the sidebar.
This patch contains a suggested solution for that, by displaying an hide-on-interaction notification on the `sidebarToggle` button (and the relevant sidebar view buttons). Note that this was inspired by e.g. the update notification that is displayed on the menu button in Firefox.
For an initial implementation, I've tried to do this in such a way that the notification isn't too distracting. Without being an UX expert, I don't think that we'd want something too in-your-face, in order to keep the viewer toolbars reasonable clean. (We probably do *not* want e.g. an entire notification bar in these situations, since that would take up unnecessary screen space and require actions from the user to close.)
However it's certainly possible that the current notification might simply be *too* inconspicuous to be truly helpful to users, but we could probably iterate on that if the feature itself is deemed useful.
This patch resolves the responsiveness issues for the toolbar in the
viewer. Depending on the language (for example the Dutch language),
elements could overlap when the viewport size is reduced.
The main issue here is that the CSS rules are unnecessarily complex and
handle lots of different cases (LTR/RTL, displacements for specific
viewport widths, et cetera). By removing this complexity and letting the
browser handle the responsiveness, we not only get simpler CSS rules and
HTML mark-up, but the responsiveness issues are mostly fixed at the same
time. We no longer have to position the elements manually (by setting
their `left` attribute value) anymore.
The following reasoning was used for deciding to remove the "Page: " label, and replace it with a tooltip, from the main toolbar:
- We have no other visible labels in the *main* toolbar (e.g. the Zoom dropdown doesn't have a label, but only a tooltip).
- We already hide the "Page: " label when the viewer is narrow.
- The varying width of the "Page: " label in different locales is already causing issues for many languages, with overlap in the main toolbar as a result.
Trying to create responsive CSS styles that works well in all locales is already difficult, and if we add support for page labels that will only further compound the issues.
- Some PDF viewers (e.g. Adobe Reader, pdfium in Chrome) doesn't show labels in the UI by default.
This patch contains an alternative solution (see the referenced PRs in 4718), in order for the indeterminate loading bar to have a consistent speed regardless of width.
Fixes 4718.
Prior to PR 6242, the width of all outline items was set such that their right (or left, in RTL locales) edges lined up vertically. In my opinion that looked more consistent, therefore this patch adjusts the CSS to make sure that this will be the case again.
The patch also makes the `border-radius` values of outline items a bit more consistent.
It appears to me that the `expanded/collapsed` part of the names got switched around, since I'd expect the following convention:
- `v` == expanded
- `>` == collapsed
