Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove the secondaryToolbarButton CSS class #18596

Merged
merged 1 commit into from
Aug 11, 2024
Merged

Remove the secondaryToolbarButton CSS class #18596

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
104 changes: 52 additions & 52 deletions web/viewer.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -629,7 +629,6 @@ body {
}

.toolbarButton,
.secondaryToolbarButton,
.dialogButton {
border: none;
background: none;
Expand All @@ -653,7 +652,7 @@ body {
overflow: hidden;
}

:is(.toolbarButton, .secondaryToolbarButton, .dialogButton)[disabled] {
:is(.toolbarButton, .dialogButton)[disabled] {
opacity: 0.5;
}

Expand All @@ -679,7 +678,6 @@ body {

.toolbarButton,
.dropdownToolbarButton,
.secondaryToolbarButton,
.dialogButton {
min-width: 16px;
margin: 2px 1px;
Expand All @@ -697,27 +695,23 @@ body {
.toolbarButton:is(:hover, :focus-visible) {
background-color: var(--button-hover-color);
}
.secondaryToolbarButton:is(:hover, :focus-visible) {
background-color: var(--doorhanger-hover-bg-color);
color: var(--doorhanger-hover-color);
}

:is(.toolbarButton, .secondaryToolbarButton).toggled,
.toolbarButton.toggled,
.splitToolbarButton.toggled > .toolbarButton.toggled {
background-color: var(--toggled-btn-bg-color);
color: var(--toggled-btn-color);
}

:is(.toolbarButton, .secondaryToolbarButton).toggled:hover,
.toolbarButton.toggled:hover,
.splitToolbarButton.toggled > .toolbarButton.toggled:hover {
outline: var(--toggled-hover-btn-outline) !important;
}

:is(.toolbarButton, .secondaryToolbarButton).toggled::before {
.toolbarButton.toggled::before {
background-color: var(--toggled-btn-color);
}

:is(.toolbarButton, .secondaryToolbarButton).toggled:hover:active,
.toolbarButton.toggled:hover:active,
.splitToolbarButton.toggled > .toolbarButton.toggled:hover:active {
background-color: var(--toggled-hover-active-btn-color);
}
Expand Down Expand Up @@ -765,7 +759,7 @@ body {
height: 1px;
}

:is(.toolbarButton, .secondaryToolbarButton, .treeItemToggler)::before,
:is(.toolbarButton, .treeItemToggler)::before,
.dropdownToolbarButton::after {
/* All matching images have a size of 16x16
* All relevant containers have a size of 28x28 */
Expand All @@ -789,17 +783,10 @@ body {
left: 6px;
}

.toolbarButton:is(:hover, :focus-visible)::before,
.secondaryToolbarButton:is(:hover, :focus-visible)::before {
.toolbarButton:is(:hover, :focus-visible)::before {
background-color: var(--toolbar-icon-hover-bg-color);
}

.secondaryToolbarButton::before {
opacity: var(--doorhanger-icon-opacity);
top: 5px;
inset-inline-start: 12px;
}

#sidebarToggle::before {
mask-image: var(--toolbarButton-sidebarToggle-icon);
transform: scaleX(var(--dir-factor));
Expand Down Expand Up @@ -850,15 +837,6 @@ body {
mask-image: var(--toolbarButton-download-icon);
}

a.secondaryToolbarButton {
padding-top: 5px;
text-decoration: none;
}
a:is(.toolbarButton, .secondaryToolbarButton)[href="#"] {
opacity: 0.5;
pointer-events: none;
}

#viewThumbnail::before {
mask-image: var(--toolbarButton-viewThumbnail-icon);
}
Expand Down Expand Up @@ -898,25 +876,6 @@ a:is(.toolbarButton, .secondaryToolbarButton)[href="#"] {
border-radius: 50%;
}

.secondaryToolbarButton {
position: relative;
margin: 0;
padding: 0 0 1px;
padding-inline-start: 36px;
height: auto;
min-height: 26px;
width: auto;
min-width: 100%;
text-align: start;
white-space: normal;
border-radius: 0;
box-sizing: border-box;
display: inline-block;
}
.secondaryToolbarButton > span {
padding-inline-end: 4px;
}

.verticalToolbarSeparator {
display: block;
margin: 5px 2px;
Expand Down Expand Up @@ -1264,6 +1223,47 @@ dialog :link {
}

#secondaryToolbar {
.toolbarButton {
border-radius: 0;
display: inline-block;
height: auto;
margin: 0;
padding: 0 0 1px;
padding-inline-start: 36px;
position: relative;
min-height: 26px;
min-width: 100%;
text-align: start;
white-space: normal;
width: auto;

&::before {
inset-inline-start: 12px;
opacity: var(--doorhanger-icon-opacity);
top: 5px;
}

&:not(.toggled):is(:hover, :focus-visible) {
background-color: var(--doorhanger-hover-bg-color);
color: var(--doorhanger-hover-color);
}

> span {
display: unset;
padding-inline-end: 4px;
}
}

a.toolbarButton {
Copy link
Collaborator

@Snuffleupagus Snuffleupagus Aug 11, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could this be moved into the .toolbarButton block above?

Copy link
Contributor Author

@timvandermeij timvandermeij Aug 11, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That'd be nice, but it'd basically require a way to, in nested CSS, detect on which element the class is being applied. I don't think there is way to do that (or at least I'm not aware of it and haven't been able to find anything about it online).

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be abusing notation a tiny bit, but the following does seem to work:

diff --git a/web/viewer.css b/web/viewer.css
index 7b1236e10..5da060f81 100644
--- a/web/viewer.css
+++ b/web/viewer.css
@@ -1252,15 +1252,15 @@ dialog :link {
       display: unset;
       padding-inline-end: 4px;
     }
-  }

-  a.toolbarButton {
-    padding-top: 5px;
-    text-decoration: none;
+    &:is(a) {
+      padding-top: 5px;
+      text-decoration: none;

-    &[href="#"] {
-      opacity: 0.5;
-      pointer-events: none;
+      &[href="#"] {
+        opacity: 0.5;
+        pointer-events: none;
+      }
     }
   }

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks; I'll give that a try and prepare a follow-up for this and another thing I noticed along the way.

padding-top: 5px;
text-decoration: none;

&[href="#"] {
opacity: 0.5;
pointer-events: none;
}
}

#secondaryToolbarButtonContainer {
margin-bottom: -4px;
max-height: calc(var(--viewer-container-height) - 40px);
Expand Down Expand Up @@ -1540,7 +1540,7 @@ dialog :link {
}

.visibleMediumView {
display: none;
display: none !important;
timvandermeij marked this conversation as resolved.
Show resolved Hide resolved
}

@media all and (max-width: 900px) {
Expand All @@ -1567,17 +1567,17 @@ dialog :link {
--editor-toolbar-base-offset: 40px;
}
#outerContainer .hiddenMediumView {
display: none;
display: none !important;
}
#outerContainer .visibleMediumView {
display: inherit;
display: inherit !important;
}
}

@media all and (max-width: 690px) {
.hiddenSmallView,
.hiddenSmallView * {
display: none;
display: none !important;
}
.toolbarButtonSpacer {
width: 0;
Expand Down
42 changes: 21 additions & 21 deletions web/viewer.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,7 @@

<div class="editorParamsToolbar hidden doorHangerRight" id="editorStampParamsToolbar">
<div class="editorParamsToolbarContainer">
<button id="editorStampAddImage" class="secondaryToolbarButton" type="button" title="Add image" tabindex="108" data-l10n-id="pdfjs-editor-stamp-add-image-button">
<button id="editorStampAddImage" class="toolbarButton" type="button" title="Add image" tabindex="108" data-l10n-id="pdfjs-editor-stamp-add-image-button">
<span class="editorParamsLabel" data-l10n-id="pdfjs-editor-stamp-add-image-button-label">Add image</span>
</button>
</div>
Expand All @@ -236,16 +236,16 @@
<div id="secondaryToolbar" class="secondaryToolbar hidden doorHangerRight">
<div id="secondaryToolbarButtonContainer">
<!--#if GENERIC-->
<button id="secondaryOpenFile" class="secondaryToolbarButton" type="button" title="Open File" tabindex="51" data-l10n-id="pdfjs-open-file-button">
<button id="secondaryOpenFile" class="toolbarButton" type="button" title="Open File" tabindex="51" data-l10n-id="pdfjs-open-file-button">
<span data-l10n-id="pdfjs-open-file-button-label">Open</span>
</button>
<!--#endif-->

<button id="secondaryPrint" class="secondaryToolbarButton visibleMediumView" type="button" title="Print" tabindex="52" data-l10n-id="pdfjs-print-button">
<button id="secondaryPrint" class="toolbarButton visibleMediumView" type="button" title="Print" tabindex="52" data-l10n-id="pdfjs-print-button">
<span data-l10n-id="pdfjs-print-button-label">Print</span>
</button>

<button id="secondaryDownload" class="secondaryToolbarButton visibleMediumView" type="button" title="Save" tabindex="53" data-l10n-id="pdfjs-save-button">
<button id="secondaryDownload" class="toolbarButton visibleMediumView" type="button" title="Save" tabindex="53" data-l10n-id="pdfjs-save-button">
<span data-l10n-id="pdfjs-save-button-label">Save</span>
</button>

Expand All @@ -255,82 +255,82 @@
<!-- <div class="horizontalToolbarSeparator visibleMediumView"></div>-->
<!--#endif-->

<button id="presentationMode" class="secondaryToolbarButton" type="button" title="Switch to Presentation Mode" tabindex="54" data-l10n-id="pdfjs-presentation-mode-button">
<button id="presentationMode" class="toolbarButton" type="button" title="Switch to Presentation Mode" tabindex="54" data-l10n-id="pdfjs-presentation-mode-button">
<span data-l10n-id="pdfjs-presentation-mode-button-label">Presentation Mode</span>
</button>

<a href="#" id="viewBookmark" class="secondaryToolbarButton" title="Current Page (View URL from Current Page)" tabindex="55" data-l10n-id="pdfjs-bookmark-button">
<a href="#" id="viewBookmark" class="toolbarButton" title="Current Page (View URL from Current Page)" tabindex="55" data-l10n-id="pdfjs-bookmark-button">
<span data-l10n-id="pdfjs-bookmark-button-label">Current Page</span>
</a>

<div id="viewBookmarkSeparator" class="horizontalToolbarSeparator"></div>

<button id="firstPage" class="secondaryToolbarButton" type="button" title="Go to First Page" tabindex="56" data-l10n-id="pdfjs-first-page-button">
<button id="firstPage" class="toolbarButton" type="button" title="Go to First Page" tabindex="56" data-l10n-id="pdfjs-first-page-button">
<span data-l10n-id="pdfjs-first-page-button-label">Go to First Page</span>
</button>
<button id="lastPage" class="secondaryToolbarButton" type="button" title="Go to Last Page" tabindex="57" data-l10n-id="pdfjs-last-page-button">
<button id="lastPage" class="toolbarButton" type="button" title="Go to Last Page" tabindex="57" data-l10n-id="pdfjs-last-page-button">
<span data-l10n-id="pdfjs-last-page-button-label">Go to Last Page</span>
</button>

<div class="horizontalToolbarSeparator"></div>

<button id="pageRotateCw" class="secondaryToolbarButton" type="button" title="Rotate Clockwise" tabindex="58" data-l10n-id="pdfjs-page-rotate-cw-button">
<button id="pageRotateCw" class="toolbarButton" type="button" title="Rotate Clockwise" tabindex="58" data-l10n-id="pdfjs-page-rotate-cw-button">
<span data-l10n-id="pdfjs-page-rotate-cw-button-label">Rotate Clockwise</span>
</button>
<button id="pageRotateCcw" class="secondaryToolbarButton" type="button" title="Rotate Counterclockwise" tabindex="59" data-l10n-id="pdfjs-page-rotate-ccw-button">
<button id="pageRotateCcw" class="toolbarButton" type="button" title="Rotate Counterclockwise" tabindex="59" data-l10n-id="pdfjs-page-rotate-ccw-button">
<span data-l10n-id="pdfjs-page-rotate-ccw-button-label">Rotate Counterclockwise</span>
</button>

<div class="horizontalToolbarSeparator"></div>

<div id="cursorToolButtons" role="radiogroup">
<button id="cursorSelectTool" class="secondaryToolbarButton toggled" type="button" title="Enable Text Selection Tool" tabindex="60" data-l10n-id="pdfjs-cursor-text-select-tool-button" role="radio" aria-checked="true">
<button id="cursorSelectTool" class="toolbarButton toggled" type="button" title="Enable Text Selection Tool" tabindex="60" data-l10n-id="pdfjs-cursor-text-select-tool-button" role="radio" aria-checked="true">
<span data-l10n-id="pdfjs-cursor-text-select-tool-button-label">Text Selection Tool</span>
</button>
<button id="cursorHandTool" class="secondaryToolbarButton" type="button" title="Enable Hand Tool" tabindex="61" data-l10n-id="pdfjs-cursor-hand-tool-button" role="radio" aria-checked="false">
<button id="cursorHandTool" class="toolbarButton" type="button" title="Enable Hand Tool" tabindex="61" data-l10n-id="pdfjs-cursor-hand-tool-button" role="radio" aria-checked="false">
<span data-l10n-id="pdfjs-cursor-hand-tool-button-label">Hand Tool</span>
</button>
</div>

<div class="horizontalToolbarSeparator"></div>

<div id="scrollModeButtons" role="radiogroup">
<button id="scrollPage" class="secondaryToolbarButton" type="button" title="Use Page Scrolling" tabindex="62" data-l10n-id="pdfjs-scroll-page-button" role="radio" aria-checked="false">
<button id="scrollPage" class="toolbarButton" type="button" title="Use Page Scrolling" tabindex="62" data-l10n-id="pdfjs-scroll-page-button" role="radio" aria-checked="false">
<span data-l10n-id="pdfjs-scroll-page-button-label">Page Scrolling</span>
</button>
<button id="scrollVertical" class="secondaryToolbarButton toggled" type="button" title="Use Vertical Scrolling" tabindex="63" data-l10n-id="pdfjs-scroll-vertical-button" role="radio" aria-checked="true">
<button id="scrollVertical" class="toolbarButton toggled" type="button" title="Use Vertical Scrolling" tabindex="63" data-l10n-id="pdfjs-scroll-vertical-button" role="radio" aria-checked="true">
<span data-l10n-id="pdfjs-scroll-vertical-button-label" >Vertical Scrolling</span>
</button>
<button id="scrollHorizontal" class="secondaryToolbarButton" type="button" title="Use Horizontal Scrolling" tabindex="64" data-l10n-id="pdfjs-scroll-horizontal-button" role="radio" aria-checked="false">
<button id="scrollHorizontal" class="toolbarButton" type="button" title="Use Horizontal Scrolling" tabindex="64" data-l10n-id="pdfjs-scroll-horizontal-button" role="radio" aria-checked="false">
<span data-l10n-id="pdfjs-scroll-horizontal-button-label">Horizontal Scrolling</span>
</button>
<button id="scrollWrapped" class="secondaryToolbarButton" type="button" title="Use Wrapped Scrolling" tabindex="65" data-l10n-id="pdfjs-scroll-wrapped-button" role="radio" aria-checked="false">
<button id="scrollWrapped" class="toolbarButton" type="button" title="Use Wrapped Scrolling" tabindex="65" data-l10n-id="pdfjs-scroll-wrapped-button" role="radio" aria-checked="false">
<span data-l10n-id="pdfjs-scroll-wrapped-button-label">Wrapped Scrolling</span>
</button>
</div>

<div class="horizontalToolbarSeparator"></div>

<div id="spreadModeButtons" role="radiogroup">
<button id="spreadNone" class="secondaryToolbarButton toggled" type="button" title="Do not join page spreads" tabindex="66" data-l10n-id="pdfjs-spread-none-button" role="radio" aria-checked="true">
<button id="spreadNone" class="toolbarButton toggled" type="button" title="Do not join page spreads" tabindex="66" data-l10n-id="pdfjs-spread-none-button" role="radio" aria-checked="true">
<span data-l10n-id="pdfjs-spread-none-button-label">No Spreads</span>
</button>
<button id="spreadOdd" class="secondaryToolbarButton" type="button" title="Join page spreads starting with odd-numbered pages" tabindex="67" data-l10n-id="pdfjs-spread-odd-button" role="radio" aria-checked="false">
<button id="spreadOdd" class="toolbarButton" type="button" title="Join page spreads starting with odd-numbered pages" tabindex="67" data-l10n-id="pdfjs-spread-odd-button" role="radio" aria-checked="false">
<span data-l10n-id="pdfjs-spread-odd-button-label">Odd Spreads</span>
</button>
<button id="spreadEven" class="secondaryToolbarButton" type="button" title="Join page spreads starting with even-numbered pages" tabindex="68" data-l10n-id="pdfjs-spread-even-button" role="radio" aria-checked="false">
<button id="spreadEven" class="toolbarButton" type="button" title="Join page spreads starting with even-numbered pages" tabindex="68" data-l10n-id="pdfjs-spread-even-button" role="radio" aria-checked="false">
<span data-l10n-id="pdfjs-spread-even-button-label">Even Spreads</span>
</button>
</div>

<div id="imageAltTextSettingsSeparator" class="horizontalToolbarSeparator hidden"></div>
<button id="imageAltTextSettings" type="button" class="secondaryToolbarButton hidden" title="Image alt text settings" tabindex="69" data-l10n-id="pdfjs-image-alt-text-settings-button" aria-controls="altTextSettingsDialog">
<button id="imageAltTextSettings" type="button" class="toolbarButton hidden" title="Image alt text settings" tabindex="69" data-l10n-id="pdfjs-image-alt-text-settings-button" aria-controls="altTextSettingsDialog">
<span data-l10n-id="pdfjs-image-alt-text-settings-button-label">Image alt text settings</span>
</button>

<div class="horizontalToolbarSeparator"></div>

<button id="documentProperties" class="secondaryToolbarButton" type="button" title="Document Properties…" tabindex="70" data-l10n-id="pdfjs-document-properties-button" aria-controls="documentPropertiesDialog">
<button id="documentProperties" class="toolbarButton" type="button" title="Document Properties…" tabindex="70" data-l10n-id="pdfjs-document-properties-button" aria-controls="documentPropertiesDialog">
<span data-l10n-id="pdfjs-document-properties-button-label">Document Properties…</span>
</button>
</div>
Expand Down