Move 'presentation mode' and 'bookmarks' buttons in the secondary toolbar (bug 1789082)

[calixteman]

No description provided.

[Snuffleupagus]

Do we have any data that says that this functionality is used sparingly, or how was this decision made (since there's no additional context in the bug)?
To me, it feels like this will result in (potentially lots of) bug reports from users who now can't (easily) find these buttons.

[Snuffleupagus]

Given that the existing code should be working, I don't understand what this change is actually for?

[calixteman]

The highlight was not correct when hovering the menu item:

If there is a better fix, please tell me.

[Snuffleupagus]

This might be a perfectly fine change, but I'd like to understand why it's necessary since it wasn't in the old code (when the button became visible at a small viewer-width).

Could it possibly have been that the following rule "fixed" that, and thus obscured that this was always broken?

pdf.js/web/viewer.css

Lines 1506 to 1508 in 50d72fc

	.visibleSmallView {
	display: inherit;
	}

[calixteman]

The default display value for a <button> is inline-block and inline for a <a>.
When we're in the state you mentioned, inherit will give the display value of the containing div which is by default block.

My feeling is that it's a bit messy and we should likely be more explicit in specifying that all the direct children of the secondary toolbar should be block or inline-block, ... something like that.
Anyway, I'll remove this "fix" from this patch because it's unrelated and we should fix that stuff in an other patch.

[calixteman]

Do we have any data that says that this functionality is used sparingly, or how was this decision made (since there's no additional context in the bug)? To me, it feels like this will result in (potentially lots of) bug reports from users who now can't (easily) find these buttons.

@rtestard ^^

[Snuffleupagus]

Also, doesn't this need to be updated as follows to prevent a useless separator at narrow viewer-widths?

Suggested change

	<div class="verticalToolbarSeparator hiddenSmallView"></div>
	<div class="verticalToolbarSeparator hiddenMediumView"></div>

[rtestard]

Ideally we would have had data points but we don't have them at this point - given we're landing an additional 2 toolbar buttons we had to make a call about increasing toolbar complexity without further considerations or moving some items that feel lower value to the overflow. This change feels low risk because:

• the bookmarks button is a pretty hard to understand/discover feature. I personally struggled to find people who understood what it does. I think this is valuable but would deserve better onboarding for users to understand what this is about. In any case it seems very low risk to move to overflow for now and better UI since retaining it seems to confuse users more than benefit them given it's just not obvious what the feature does even if you click the button.
• presentation mode is also available through the browser full screen feature / F11 even though the pdf.js toolbar persists. Rather than having 2 separate entry points for a single feature it feels like we should let users rely on the browser full screen feature as opposed to having 2 separate entry points for very similar capabilities. We'll need to decide if we want to retain full screen with pdf.js toolbar and full screen without pdf.js toolbar (i.e keep both browser full screen and pdf.js full screen implementation) or just retain browser full screen with a change allowing to mask the pdf.js toolbar when in browser full screen. This last option feels more suited to me but we'll need to get UX input on this

In general users don't benefit from having too many toolbar options, especially when some are redundant or very corner case. These options will still be available from the overflow menu.

[Snuffleupagus]

Ideally we would have had data points but we don't have them at this point

At least for PresentationMode, it'd probably not be that difficult to add telemetry for it (based on how "simple" the editing-telemetry was to add).

presentation mode is also available through the browser full screen feature / F11 even though the pdf.js toolbar persists.

Those two modes are not identical though, quite far from it, and I've always felt that they rather complement each other.

Rather than having 2 separate entry points for a single feature it feels like we should let users rely on the browser full screen feature as opposed to having 2 separate entry points for very similar capabilities.

Note that doing so would lose the presentation-feature, which exists as a simple way to show a PDF document one page at a time without any background-colour. (You could partially simulate that by manually setting an appropriate scroll-mode, additionally when the browser uses a light theme you'd then have a light background around the pages.)

In general users don't benefit from having too many toolbar options, especially when some are redundant or very corner case. These options will still be available from the overflow menu.

That might very well be true for the "average" user (if such a term is even appropriate/meaningful to use), however note that there's actually been (old) feature requests from users that wanted more of buttons available in the main toolbar :-)

[Snuffleupagus]

Following up on #15391 (review): I just saw https://www.reddit.com/r/firefox/comments/zcy0qf/no_more_pdf_bookmark_option/ which means that there's definitely users that relied on the viewBookmark button and are now having difficulty finding it.

[calixteman]

@rtestard ^^

[rtestard]

Apologies for delay.
We now have telemetry for all overflow items.
73% of use among overflow items is for rotation (clockwise and counter clockwise), third is document propertie, fourth is presentation mode (5% of use) and view bookmark is 7th with 2% of use.
Overall 0.05% of daily PDF viewers use presentation mode and 0.03% of daily PDF viewers use view bookmark.
I agree that the change would have regressed the experience for some users but these numbers clearly show that they are a minority - per comments above we cannot retain low use features in primary UI since this tends to confuse users and make other more frequently used features harder to find when needed. I hope this helps gain confidence in our approach.

[Snuffleupagus]

We now have telemetry for all overflow items.

While I suppose that it might not change things all that much, there's however one general problem with those measurements (from a statistical point of view).
Given that the telemetry was add after the PresentationMode/ViewBookmark buttons were already moved, it's possible that that skewed the results downwards since some users may simply not find those buttons now (as evident from the Reddit thread).

Edit: Basically, I'm just saying that you need to be somewhat careful interpreting the results because of this.