Skip to content

Commit 1db3774

Browse files
calixtemancalixteman
committed
Make sure the menuitems are readable when hovering them (bug 2023138)
Simplify a bit the html for the menu: remove the li elements. Make sure the the menuitems are exposed when disabled.
1 parent 746e6b4 commit 1db3774

3 files changed

Lines changed: 85 additions & 114 deletions

File tree

‎web/menu.css‎

Lines changed: 60 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -105,10 +105,10 @@ button.hasPopupMenu {
105105
--menu-mark-icon-size: 16px;
106106
}
107107

108-
> li {
108+
> button {
109109
display: flex;
110110
align-items: center;
111-
list-style: none;
111+
position: relative;
112112
width: 100%;
113113
height: 26px;
114114
padding-inline: calc(var(--menu-mark-icon-size) + var(--menuitem-gap))
@@ -119,7 +119,7 @@ button.hasPopupMenu {
119119
border: 1px solid var(--menuitem-border-color);
120120
background: transparent;
121121

122-
&:has(button.selected)::before {
122+
&.selected::after {
123123
content: "";
124124
display: inline-block;
125125
width: 11px;
@@ -129,101 +129,88 @@ button.hasPopupMenu {
129129
mask-image: var(--menuitem-checkmark-icon);
130130
background-color: var(--menu-text-color);
131131
position: absolute;
132-
margin-inline-start: -16px;
132+
inset-inline-start: var(--menuitem-gap);
133+
top: 50%;
134+
transform: translateY(-50%);
133135
}
134136

135-
&:has(button:disabled) {
137+
&:disabled {
136138
opacity: var(--disabled-opacity);
137-
pointer-events: none;
138139

139-
&:has(button.selected)::before {
140+
&.selected::after {
140141
background-color: var(--menu-text-disabled-color);
141142
}
143+
144+
&:not(.noIcon)::before {
145+
background-color: var(--menu-text-disabled-color);
146+
}
147+
148+
> span {
149+
color: var(--menu-text-disabled-color);
150+
}
142151
}
143152

144-
&:hover {
145-
background: var(--menuitem-hover-bg);
146-
background-blend-mode: var(--menuitem-hover-background-blend-mode);
147-
> button {
153+
&:not(:disabled) {
154+
&:hover {
155+
background: var(--menuitem-hover-bg);
156+
background-blend-mode: var(--menuitem-hover-background-blend-mode);
157+
148158
&:not(.noIcon)::before {
149159
background-color: var(--menuitem-text-hover-fg);
150160
}
161+
151162
> span {
152163
color: var(--menuitem-text-hover-fg);
153164
}
165+
166+
&.selected::after {
167+
background-color: var(--menuitem-text-hover-fg);
168+
}
154169
}
155-
&:has(button.selected)::before {
156-
background-color: var(--menuitem-text-hover-fg);
170+
171+
&:active {
172+
background-color: var(--menuitem-active-bg);
173+
border-color: var(--menuitem-active-border-color);
174+
175+
> span {
176+
color: var(--menuitem-text-active-fg);
177+
}
157178
}
158-
}
159179

160-
&:active {
161-
background-color: var(--menuitem-active-bg);
162-
border-color: var(--menuitem-active-border-color);
163-
> button > span {
164-
color: var(--menuitem-text-active-fg);
180+
&:focus-visible {
181+
border-color: var(--menuitem-focus-border-color);
182+
background-color: var(--menuitem-focus-bg);
183+
outline: 2px solid var(--menuitem-focus-outline-color);
184+
outline-offset: 2px;
165185
}
166186
}
167187

168-
&:has(> button:focus-visible) {
169-
border-color: var(--menuitem-focus-border-color);
170-
background-color: var(--menuitem-focus-bg);
171-
outline: 2px solid var(--menuitem-focus-outline-color);
172-
outline-offset: 2px;
188+
&:not(.noIcon)::before {
189+
display: inline-block;
190+
width: var(--menu-icon-size);
191+
height: var(--menu-icon-size);
192+
content: "";
193+
mask-size: cover;
194+
mask-position: center;
195+
background-color: var(--menu-text-color);
173196
}
174197

175-
> button {
198+
> span {
176199
display: flex;
177-
flex-direction: row;
178200
align-items: center;
201+
align-self: stretch;
179202
width: 100%;
180-
height: auto;
181-
padding: var(--menuitem-gap);
182-
gap: var(--menuitem-gap);
183-
background: transparent;
184-
border: none;
185-
186-
&:not(.noIcon)::before {
187-
display: inline-block;
188-
width: var(--menu-icon-size);
189-
height: var(--menu-icon-size);
190-
content: "";
191-
mask-size: cover;
192-
mask-position: center;
193-
background-color: var(--menu-text-color);
194-
}
195-
196-
&:focus-visible {
197-
outline: none;
198-
}
199-
200-
> span {
201-
display: inline-block;
202-
width: max-content;
203-
height: auto;
204-
text-align: start;
205-
color: var(--menu-text-color);
206-
user-select: none;
207-
padding-inline-start: 6px;
208-
209-
font: menu;
210-
font-size: 13px;
211-
font-style: normal;
212-
font-weight: 510;
213-
line-height: normal;
214-
}
215-
216-
&:disabled {
217-
pointer-events: none;
218-
219-
&:not(.noIcon)::before {
220-
background-color: var(--menu-text-disabled-color);
221-
}
222-
223-
> span {
224-
color: var(--menu-text-disabled-color);
225-
}
226-
}
203+
min-width: max-content;
204+
text-align: start;
205+
color: var(--menu-text-color);
206+
user-select: none;
207+
padding-inline-start: 6px;
208+
209+
font: menu;
210+
font-size: 13px;
211+
font-style: normal;
212+
font-weight: 510;
213+
line-height: normal;
227214
}
228215
}
229216
}

‎web/viewer.html‎

Lines changed: 24 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -142,26 +142,18 @@
142142
<span data-l10n-id="pdfjs-views-manager-view-selector-button-label"></span>
143143
</button>
144144
<menu id="viewsManagerSelectorOptions" role="listbox" class="popupMenu withMark">
145-
<li>
146-
<button id="thumbnailsViewMenu" role="option" type="button" tabindex="-1">
147-
<span data-l10n-id="pdfjs-views-manager-pages-option-label"></span>
148-
</button>
149-
</li>
150-
<li>
151-
<button id="outlinesViewMenu" role="option" type="button" tabindex="-1">
152-
<span data-l10n-id="pdfjs-views-manager-outlines-option-label"></span>
153-
</button>
154-
</li>
155-
<li>
156-
<button id="attachmentsViewMenu" role="option" type="button" tabindex="-1">
157-
<span data-l10n-id="pdfjs-views-manager-attachments-option-label"></span>
158-
</button>
159-
</li>
160-
<li>
161-
<button id="layersViewMenu" role="option" type="button" tabindex="-1">
162-
<span data-l10n-id="pdfjs-views-manager-layers-option-label"></span>
163-
</button>
164-
</li>
145+
<button id="thumbnailsViewMenu" role="option" type="button" tabindex="-1">
146+
<span data-l10n-id="pdfjs-views-manager-pages-option-label"></span>
147+
</button>
148+
<button id="outlinesViewMenu" role="option" type="button" tabindex="-1">
149+
<span data-l10n-id="pdfjs-views-manager-outlines-option-label"></span>
150+
</button>
151+
<button id="attachmentsViewMenu" role="option" type="button" tabindex="-1">
152+
<span data-l10n-id="pdfjs-views-manager-attachments-option-label"></span>
153+
</button>
154+
<button id="layersViewMenu" role="option" type="button" tabindex="-1">
155+
<span data-l10n-id="pdfjs-views-manager-layers-option-label"></span>
156+
</button>
165157
</menu>
166158
</div>
167159
<span id="viewsManagerHeaderLabel" class="viewsManagerLabel" role="heading" aria-level="2"></span>
@@ -217,26 +209,18 @@
217209
<span data-l10n-id="pdfjs-views-manager-pages-status-action-button-label"></span>
218210
</button>
219211
<menu id="viewsManagerStatusActionOptions" class="popupMenu">
220-
<li>
221-
<button id="viewsManagerStatusActionCopy" class="noIcon" role="menuitem" type="button" tabindex="-1" disabled>
222-
<span data-l10n-id="pdfjs-views-manager-pages-status-copy-button-label"></span>
223-
</button>
224-
</li>
225-
<li>
226-
<button id="viewsManagerStatusActionCut" class="noIcon" role="menuitem" type="button" tabindex="-1" disabled>
227-
<span data-l10n-id="pdfjs-views-manager-pages-status-cut-button-label"></span>
228-
</button>
229-
</li>
230-
<li>
231-
<button id="viewsManagerStatusActionDelete" class="noIcon" role="menuitem" type="button" tabindex="-1" disabled>
232-
<span data-l10n-id="pdfjs-views-manager-pages-status-delete-button-label"></span>
233-
</button>
234-
</li>
235-
<li>
236-
<button id="viewsManagerStatusActionExport" class="noIcon" role="menuitem" type="button" tabindex="-1" disabled>
237-
<span data-l10n-id="pdfjs-views-manager-pages-status-export-selected-button-label"></span>
238-
</button>
239-
</li>
212+
<button id="viewsManagerStatusActionCopy" class="noIcon" role="menuitem" type="button" tabindex="-1" disabled>
213+
<span data-l10n-id="pdfjs-views-manager-pages-status-copy-button-label"></span>
214+
</button>
215+
<button id="viewsManagerStatusActionCut" class="noIcon" role="menuitem" type="button" tabindex="-1" disabled>
216+
<span data-l10n-id="pdfjs-views-manager-pages-status-cut-button-label"></span>
217+
</button>
218+
<button id="viewsManagerStatusActionDelete" class="noIcon" role="menuitem" type="button" tabindex="-1" disabled>
219+
<span data-l10n-id="pdfjs-views-manager-pages-status-delete-button-label"></span>
220+
</button>
221+
<button id="viewsManagerStatusActionExport" class="noIcon" role="menuitem" type="button" tabindex="-1" disabled>
222+
<span data-l10n-id="pdfjs-views-manager-pages-status-export-selected-button-label"></span>
223+
</button>
240224
</menu>
241225
</div>
242226
</div>

‎web/views_manager.css‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -337,7 +337,7 @@
337337
min-width: 182px;
338338
z-index: 1;
339339

340-
> li > button {
340+
> button {
341341
&#thumbnailsViewMenu::before {
342342
mask-image: var(--menuitem-thumbnailsView-icon);
343343
}

0 commit comments

Comments
 (0)