Skip to content

Commit db4bc49

Browse files
committed
Fixed: Use disclosure triangles for metadata blocks
1 parent 126a6f8 commit db4bc49

9 files changed

Lines changed: 249 additions & 73 deletions

File tree

browser-tests/public-api.spec.ts

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -482,6 +482,47 @@ test("bolds ranges containing either page of a two-up spread", async ({page}) =>
482482
await expect(otherRange).toHaveClass(/is-current/);
483483
});
484484

485+
test("uses separate controls for range navigation and metadata disclosure", async ({page}) => {
486+
const name = "range-disclosure";
487+
await page.route(`${origin}/api/${name}/manifest`, (route) => route.fulfill({json : {
488+
...manifest(name, 2),
489+
structures : [ {
490+
id : `${origin}/api/${name}/range/a`,
491+
type : "Range",
492+
label : {en : [ "Section A" ]},
493+
metadata : [ {label : {en : [ "Composer" ]}, value : {en : [ "Anonymous" ]}} ],
494+
items : [ {id : `${origin}/api/${name}/canvas/1`, type : "Canvas"} ]
495+
} ]
496+
}}));
497+
498+
await page.evaluate(async (url) => {
499+
const diva = (window as any).diva;
500+
await diva.setResource(url);
501+
await diva.goToPage(1);
502+
}, `${origin}/api/${name}/manifest`);
503+
await page.getByRole("button", {name : "Contents"}).evaluate((button: HTMLButtonElement) => button.click());
504+
505+
const showInformation = page.getByRole("button", {name : "Show information for Section A", exact : true});
506+
await expect(showInformation).toHaveAttribute("aria-expanded", "false");
507+
await expect(page.getByText("Anonymous", {exact : true})).toHaveCount(0);
508+
509+
await showInformation.click();
510+
await expect.poll(() => page.evaluate(() => (window as any).diva.getState().currentPageIndex)).toBe(1);
511+
const hideInformation = page.getByRole("button", {name : "Hide information for Section A", exact : true});
512+
await expect(hideInformation).toHaveAttribute("aria-expanded", "true");
513+
await expect(page.getByText("Anonymous", {exact : true})).toBeVisible();
514+
const metadataLabel = page.getByText("Composer", {exact : true});
515+
await expect(metadataLabel).toHaveCSS("font-weight", "600");
516+
await expect(metadataLabel).toHaveCSS("text-transform", "none");
517+
518+
await page.getByRole("button", {name : "Section A", exact : true}).click();
519+
await expect.poll(() => page.evaluate(() => (window as any).diva.getState().currentPageIndex)).toBe(0);
520+
await expect(page.getByText("Anonymous", {exact : true})).toBeVisible();
521+
522+
await hideInformation.click();
523+
await expect(page.getByText("Anonymous", {exact : true})).toHaveCount(0);
524+
});
525+
485526
test("indents nested ranges in the contents index", async ({page}) => {
486527
const name = "nested-ranges";
487528
await page.route(`${origin}/api/${name}/manifest`, (route) => route.fulfill({json : {

build/diva.debug.js

Lines changed: 115 additions & 55 deletions
Large diffs are not rendered by default.

build/diva.esm.js

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

build/diva.js

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/Main.elm

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1407,7 +1407,7 @@ update msg model =
14071407
UserClickedPageViewPrev ->
14081408
handlePageViewStep -1 model
14091409

1410-
UserClickedRange rangeId maybeIndex ->
1410+
UserClickedRange maybeIndex ->
14111411
let
14121412
nextModel =
14131413
{ model
@@ -1419,7 +1419,6 @@ update msg model =
14191419

14201420
Nothing ->
14211421
model.selectedIndex
1422-
, selectedRangeId = Just rangeId
14231422
, sidebarState = visibleSidebarState model
14241423
, thumbsInstantScroll = True
14251424
}
@@ -1594,6 +1593,18 @@ update msg model =
15941593
UserToggledPageViewSidebar ->
15951594
( { model | pageViewSidebarVisible = not model.pageViewSidebarVisible }, Cmd.none )
15961595

1596+
UserToggledRangeMetadata rangeId ->
1597+
( { model
1598+
| selectedRangeId =
1599+
if model.selectedRangeId == Just rangeId then
1600+
Nothing
1601+
1602+
else
1603+
Just rangeId
1604+
}
1605+
, Cmd.none
1606+
)
1607+
15971608
UserToggledShiftByOne ->
15981609
case model.viewMode of
15991610
OneUp ->

src/Msg.elm

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ type Msg
2929
| UserClickedPageViewImageChoice Int
3030
| UserClickedPageViewNext
3131
| UserClickedPageViewPrev
32-
| UserClickedRange String (Maybe Int)
32+
| UserClickedRange (Maybe Int)
3333
| UserClickedSaveFilteredImage
3434
| UserClickedThumbnail Int
3535
| UserClickedZoomIn
@@ -53,6 +53,7 @@ type Msg
5353
| UserToggledMetadata
5454
| UserToggledPageViewFullscreen
5555
| UserToggledPageViewSidebar
56+
| UserToggledRangeMetadata String
5657
| UserToggledShiftByOne
5758
| UserToggledSidebar
5859
| UserToggledThumbnails

src/View/Sidebar.elm

Lines changed: 60 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -455,7 +455,7 @@ viewOtpRangeItem model canvasLabelMap range =
455455
rangePrefix ++ labelText
456456

457457
labelNode =
458-
viewRangeButton False range.id maybeIndex resolvedLabel
458+
viewRangeButton False maybeIndex resolvedLabel
459459

460460
metadataBlock =
461461
viewRangeMetadata model.detectedLanguage range.metadata
@@ -465,16 +465,16 @@ viewOtpRangeItem model canvasLabelMap range =
465465
(labelNode :: metadataBlock)
466466

467467

468-
viewRangeButton : Bool -> String -> Maybe Int -> String -> Html Msg
469-
viewRangeButton isCurrent rangeId maybeIndex labelText =
468+
viewRangeButton : Bool -> Maybe Int -> String -> Html Msg
469+
viewRangeButton isCurrent maybeIndex labelText =
470470
button
471471
([ classList
472472
[ ( "contents-button", True )
473473
, ( "ui-button", True )
474474
, ( "is-current", isCurrent )
475475
]
476476
, type_ "button"
477-
, Events.onClick (UserClickedRange rangeId maybeIndex)
477+
, Events.onClick (UserClickedRange maybeIndex)
478478
]
479479
++ (if isCurrent then
480480
[ attribute "aria-current" "location" ]
@@ -486,6 +486,39 @@ viewRangeButton isCurrent rangeId maybeIndex labelText =
486486
[ text labelText ]
487487

488488

489+
viewRangeDisclosure : Bool -> String -> String -> Html Msg
490+
viewRangeDisclosure isExpanded rangeId labelText =
491+
button
492+
[ HA.class "contents-disclosure ui-button"
493+
, type_ "button"
494+
, attribute "aria-expanded"
495+
(if isExpanded then
496+
"true"
497+
498+
else
499+
"false"
500+
)
501+
, attribute "aria-label"
502+
((if isExpanded then
503+
"Hide information for "
504+
505+
else
506+
"Show information for "
507+
)
508+
++ labelText
509+
)
510+
, Events.onClick (UserToggledRangeMetadata rangeId)
511+
]
512+
[ text
513+
(if isExpanded then
514+
""
515+
516+
else
517+
""
518+
)
519+
]
520+
521+
489522
viewRangeItems : Model -> Dict String (Maybe Int) -> List RangeItem -> List (Html Msg)
490523
viewRangeItems model rangeIndexMap items =
491524
let
@@ -530,8 +563,8 @@ viewRangeMetadata language metadata =
530563
viewRangeNode : Model -> Dict String (Maybe Int) -> Range -> Html Msg
531564
viewRangeNode model rangeIndexMap range =
532565
let
533-
maybeIndex =
534-
lookupRangeIndex rangeIndexMap range.id
566+
isExpanded =
567+
model.selectedRangeId == Just range.id
535568

536569
labelText =
537570
extractLabelFromLanguageMap model.detectedLanguage range.label
@@ -543,26 +576,41 @@ viewRangeNode model rangeIndexMap range =
543576
else
544577
labelText
545578

579+
children =
580+
viewRangeItems model rangeIndexMap range.items
581+
546582
isCurrent =
547583
visibleCanvasIds model
548584
|> List.any (\canvasId -> rangeContainsCanvas canvasId range)
549585

586+
maybeIndex =
587+
lookupRangeIndex rangeIndexMap range.id
588+
550589
labelNode =
551-
viewRangeButton isCurrent range.id maybeIndex resolvedLabel
590+
viewRangeButton isCurrent maybeIndex resolvedLabel
591+
592+
headingNode =
593+
div
594+
[ HA.class "contents-heading" ]
595+
((if List.isEmpty range.metadata then
596+
[]
597+
598+
else
599+
[ viewRangeDisclosure isExpanded range.id resolvedLabel ]
600+
)
601+
++ [ labelNode ]
602+
)
552603

553604
metadataBlock =
554-
if model.selectedRangeId == Just range.id then
605+
if isExpanded then
555606
viewRangeMetadata model.detectedLanguage range.metadata
556607

557608
else
558609
[]
559-
560-
children =
561-
viewRangeItems model rangeIndexMap range.items
562610
in
563611
li
564612
[ HA.class "contents-item" ]
565-
(labelNode :: metadataBlock ++ children)
613+
(headingNode :: metadataBlock ++ children)
566614

567615

568616
viewSidebarPane : SidebarState -> SidebarState -> Html Msg -> Html Msg

src/styles/app.css

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,6 @@
151151
font-size: var(--diva-font-lg);
152152
font-weight: 600;
153153
color: var(--diva-text-muted);
154-
text-transform: uppercase;
155154
letter-spacing: 0.05em;
156155
line-height: 1.3;
157156
}

src/styles/sidebar.css

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -181,6 +181,22 @@
181181
margin-bottom: 6px;
182182
}
183183

184+
.contents-heading {
185+
display: flex;
186+
align-items: baseline;
187+
}
188+
189+
.contents-disclosure {
190+
flex: 0 0 1.25rem;
191+
color: var(--diva-text-muted);
192+
cursor: pointer;
193+
text-align: center;
194+
}
195+
196+
.contents-disclosure:hover {
197+
color: var(--diva-accent);
198+
}
199+
184200
.contents-meta {
185201
border: 1px solid var(--diva-dark-border);
186202
display: flex;

0 commit comments

Comments
 (0)