Skip to content

Commit a5f2853

Browse files
committed
Fix review findings in the second pass
A five-lens review of 7d7597b..2b29560, each finding checked by a skeptic, confirmed these. - Theme switch: a palette list without toggle keys draws Material's form but no label, so the switch showed and threw on click. It now shows only when a label exists, and never clicks a missing one. - Neighbour badges: a prerequisite read "Uses" while its accessible name said "used by <selection>". Badges now read in the card's own direction, "Used by <selection>" or "Uses <selection>", truncated. - Phone handle: at half its title said "Close reading", but a tap opens the full reader. It now says "Expand reading" there. - Hover: leaving a card cleared the preview on the next tick, so crossing the gap to the next card flashed the whole map back. Leaving now waits 120 ms, as entering does; blur still acts at once. - Tablet toolbar: with "Show unlinked" off, "Filter: … · linked only" and the link still overflowed the second row below about 785px. The filter now grows from 100px into the row's free space and truncates its summary rather than wrap. - Touch screens: the 44px Fit made the row taller in Map than in Results or Sources, so the head moved on every view switch. The zoom group now keeps its place, hidden, in the list views. The theme switch gets the same 44px target on coarse pointers. - 681 to 720px: the phone handle lacked the 680px block's width, border and font, so it shrank to its title and showed the button border. Its own rule now sets them. Tests: the tablet toolbar case runs at 721, 800 and 1000px with the longest summary; a new browser test moves the mouse from one card to the next and checks that no card outside both neighbourhoods lights in between; the five-area test checks what the area meta and summary show, not only their text; search results are checked for their 576px minimum; the four unbounded handle loops are bounded and end in an assertion.
1 parent 47c045b commit a5f2853

7 files changed

Lines changed: 113 additions & 37 deletions

‎autoform_cli/assets/blueprint-dag.css‎

Lines changed: 10 additions & 4 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎autoform_cli/assets/blueprint-dag.js‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -698,11 +698,12 @@
698698
head.appendChild(context);
699699
// The explorer hides Material's header and its sun or moon, so this clicks that toggle: the one label Material leaves
700700
// unhidden, which points at the next palette entry, so a three-entry automatic palette steps on as Material's own does.
701+
// A palette list without toggle keys draws the form but no label, so it gets no switch.
701702
var schemes = document.querySelector("[data-md-component=palette]");
702-
if (schemes) {
703+
if (schemes && schemes.querySelector("label")) {
703704
var themeLabel = "Switch light or dark theme", flip = button("bp-dag-button bp-dag-theme", "◐", themeLabel);
704705
flip.title = themeLabel; context.appendChild(flip);
705-
flip.onclick = function () { schemes.querySelector("label:not([hidden])").click(); };
706+
flip.onclick = function () { var next = schemes.querySelector("label:not([hidden])"); if (next) next.click(); };
706707
}
707708

708709
var toolbar = element("div", "bp-dag-toolbar");
@@ -1369,7 +1370,7 @@
13691370
});
13701371
if (control.getAttribute("aria-label") !== named.join(", ")) control.setAttribute("aria-label", named.join(", "));
13711372
var relationLabel = control.querySelector(".bp-dag-node-relation");
1372-
if (relationLabel) relationLabel.textContent = relation === "prerequisite" ? "Uses" : relation === "dependent" ? "Used by" : "";
1373+
if (relationLabel) relationLabel.textContent = relation ? (relation === "prerequisite" ? "Used by " : "Uses ") + ui$selected.title : "";
13731374
control.dataset.subdued = String(Boolean(spotlight && !spotlight.nodes.has(node.id)));
13741375
control.tabIndex = node.id === tabId ? 0 : -1;
13751376
});
@@ -1411,11 +1412,11 @@
14111412
// A drag or pinch that began on the card is not a tap; a keyboard click has no detail and always opens it.
14121413
control.addEventListener("click", function (event) { event.stopPropagation(); if (!ui$moved || !event.detail) activateNode(node); });
14131414
// Resting a mouse on a card for 120 ms, or focusing it from the keyboard, previews its neighbours without selecting it; touch has no hover.
1414-
// Moving straight onto another card hands over after its delay instead of flashing the whole map back.
1415+
// Leaving waits as long, so a mouse crossing the gap to another card hands over after its delay instead of flashing the whole map back.
14151416
["pointerenter", "focus", "pointerleave", "blur"].forEach(function (type, index) {
14161417
control.addEventListener(type, function (event) {
14171418
clearTimeout(hoverTimer);
1418-
if (index - 1 ? index || event.pointerType !== "touch" : control.matches(":focus-visible")) hoverTimer = setTimeout(function () { ui$hovered = index < 2 && node; schedule(); }, index ? 0 : 120);
1419+
if (index - 1 ? index || event.pointerType !== "touch" : control.matches(":focus-visible")) hoverTimer = setTimeout(function () { ui$hovered = index < 2 && node; schedule(); }, index % 2 ? 0 : 120);
14191420
});
14201421
});
14211422
control.addEventListener("keydown", function (event) {
@@ -1628,7 +1629,7 @@
16281629
record = records.get(currentBundle && currentBundle.nodeId || ui$requestedNode || (ui$selected && ui$selected.id));
16291630
dom$sheetHandle.replaceChildren(close ? "×" : record && record.status ? swatch(record.status) : "", close ? "" : title);
16301631
dom$sheetHandle.setAttribute("aria-label", close ? "Close reading" : title + ", reading");
1631-
dom$sheetHandle.title = opened ? "Close reading" : "Open reading";
1632+
dom$sheetHandle.title = !opened ? "Open reading" : navigation && sheetLayout() && !host.classList.contains("bp-dag-sheet-full") ? "Expand reading" : "Close reading";
16321633
if (collapsed) {
16331634
if (dom$inspectorInner.contains(document.activeElement)) dom$sheetHandle.focus();
16341635
dom$inspectorInner.setAttribute("inert", ""); dom$inspectorInner.setAttribute("aria-hidden", "true");

‎autoform_cli/assets/blueprint-explorer.min.js‎

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎tests/test_dag_viewer_browser.py‎

Lines changed: 46 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1020,6 +1020,44 @@ def test_resting_the_mouse_on_a_card_previews_its_neighbours_without_selecting_i
10201020
playwright.expect(foundation).to_have_attribute("data-subdued", "false")
10211021

10221022

1023+
def test_moving_the_mouse_to_the_next_card_hands_the_preview_over_without_a_flash(
1024+
desktop_page: object, explorer_consumer: _ExplorerConsumer,
1025+
) -> None:
1026+
"""Leaving a card cleared its preview on the next tick, so crossing the gap to the next card flashed the whole map back."""
1027+
page = desktop_page
1028+
page.set_viewport_size({"width": 1440, "height": 900})
1029+
_open_consumer(page, explorer_consumer, "star")
1030+
# Every chapter of the star depends on chapter 0 alone, so under two neighbouring leaves' previews every other leaf stays faded.
1031+
leaves = page.evaluate("""() => [...document.querySelectorAll('.bp-dag-node')].map(node => {
1032+
const box = node.getBoundingClientRect(), stage = node.closest('.bp-dag-stage').getBoundingClientRect();
1033+
return {id: node.dataset.autoformNodeId, x: box.left, y: box.top, w: box.width, h: box.height,
1034+
whole: box.left >= stage.left && box.right <= stage.right && box.top >= stage.top && box.bottom <= stage.bottom};
1035+
}).filter(card => card.whole && !card.id.endsWith('/chapter-00'))""")
1036+
leaves.sort(key=lambda card: (round(card["x"]), card["y"]))
1037+
pairs = [(a, b) for a, b in zip(leaves, leaves[1:]) if abs(a["x"] - b["x"]) < 1]
1038+
assert pairs and len(leaves) > 2, leaves
1039+
first, second = pairs[0]
1040+
outside = [card["id"] for card in leaves if card["id"] not in (first["id"], second["id"])]
1041+
card = page.locator(f'.bp-dag-node[data-autoform-node-id="{first["id"]}"]')
1042+
_assert_hit(page, card)
1043+
_assert_hit(page, page.locator(f'.bp-dag-node[data-autoform-node-id="{second["id"]}"]'))
1044+
card.hover()
1045+
playwright.expect(page.locator(f'.bp-dag-node[data-autoform-node-id="{outside[0]}"]')).to_have_attribute("data-subdued", "true")
1046+
page.evaluate("""ids => {
1047+
window.__lit = new Set();
1048+
window.__watch = new MutationObserver(records => records.forEach(record => {
1049+
if (record.target.dataset.subdued === "false") window.__lit.add(record.target.dataset.autoformNodeId);
1050+
}));
1051+
ids.forEach(id => window.__watch.observe(document.querySelector(`.bp-dag-node[data-autoform-node-id="${id}"]`),
1052+
{attributes: true, attributeFilter: ["data-subdued"]}));
1053+
}""", outside)
1054+
page.mouse.move(second["x"] + second["w"] / 2, second["y"] + second["h"] / 2, steps=8)
1055+
# The first leaf fades once the second's preview takes over.
1056+
playwright.expect(card).to_have_attribute("data-subdued", "true")
1057+
lit = page.evaluate("() => { window.__watch.disconnect(); return [...window.__lit]; }")
1058+
assert lit == [], f"Cards outside both neighbourhoods lit while the mouse crossed the gap: {lit}"
1059+
1060+
10231061
def test_search_keyboard_navigation_and_focus(desktop_page: object, explorer_consumer: _ExplorerConsumer) -> None:
10241062
page = desktop_page
10251063
_open_consumer(page, explorer_consumer)
@@ -1523,6 +1561,7 @@ def lengthen_the_index(route: object) -> None:
15231561
assert box is not None
15241562
assert 0 <= box["x"] and box["x"] + box["width"] <= viewport["width"]
15251563
assert box["y"] + box["height"] <= viewport["height"]
1564+
assert box["width"] >= viewport["width"] - 24 - 1 # min(576px, 100vw - 24px)
15261565
_assert_hit(page, page.locator(".bp-dag-result").first)
15271566
# Where the results reach the half-open reader, they paint over it.
15281567
assert page.evaluate(
@@ -1720,6 +1759,9 @@ def lengthen_the_index(route: object) -> None:
17201759
search.fill("Result")
17211760
playwright.expect(page.locator(".bp-dag-search-results")).to_be_visible()
17221761
assert _paints_over_the_details(page, ".bp-dag-result", ".bp-dag-search-results") is True
1762+
# Results read at least 576px wide whatever the box beside them, and stay on screen.
1763+
box = page.locator(".bp-dag-search-results").bounding_box()
1764+
assert box is not None and box["width"] >= 576 - 1 and box["x"] + box["width"] <= width + 1
17231765

17241766

17251767
def test_a_short_phones_explorer_keeps_its_sheet_handle_on_screen(mobile_page: object, site_url: str) -> None:
@@ -2486,7 +2528,7 @@ def test_the_toolbar_counts_only_what_it_hides_and_shows_it_on_request(
24862528
playwright.expect(_reading_card(page, "chapter/target")).to_contain_text("Final sentence of the complete target statement.")
24872529

24882530

2489-
@pytest.mark.parametrize("width", (800, 1000))
2531+
@pytest.mark.parametrize("width", (721, 800, 1000))
24902532
def test_a_filter_never_adds_a_tablet_toolbar_row(desktop_page: object, explorer_consumer: _ExplorerConsumer, width: int) -> None:
24912533
"""The "Show N filtered out" link wrapped onto a row of its own at 1000px and pushed the map down."""
24922534
page = desktop_page
@@ -2496,6 +2538,9 @@ def test_a_filter_never_adds_a_tablet_toolbar_row(desktop_page: object, explorer
24962538
before = toolbar.bounding_box(), stage.bounding_box()
24972539
page.locator(".bp-dag-filter-summary").click()
24982540
page.locator('.bp-dag-filter-popover input[value="can_prove"]').uncheck()
2541+
# The longest summary, "Filter: 5 of 6 statuses · linked only", gives way to the link rather than wrap it.
2542+
page.get_by_role("checkbox", name="Show unlinked", exact=True).uncheck()
2543+
playwright.expect(page.locator(".bp-dag-filter-summary")).to_have_text(re.compile(r"linked only$"))
24992544
playwright.expect(page.locator(".bp-dag-hidden-button")).to_be_visible()
25002545
assert (toolbar.bounding_box()["height"], stage.bounding_box()["y"]) == (before[0]["height"], before[1]["y"])
25012546

0 commit comments

Comments
 (0)