fix(ui): the review-pass fixes — 7 issues across chrome, dialogs, and the guided editor - #558
Open
angela-helios wants to merge 7 commits into
Open
fix(ui): the review-pass fixes — 7 issues across chrome, dialogs, and the guided editor#558angela-helios wants to merge 7 commits into
angela-helios wants to merge 7 commits into
Conversation
…er layout Three findings from the UI review pass, one PR since they share the chrome: - The sidebar tenant picker rendered on every install, offering exactly one choice on single-tenant servers. It now renders only when the effective tenant differs from the default or the registry knows a second tenant -- one indexed registry read per page load, the same cost class as the settings read the middleware already does (#544). - The Add-tenant panel (and the bulk import dialogs) could not be backed out of: no close control, no Esc, no click-outside. A small addbox.js asset adds all three ways out -- Esc, a click outside the panel, and explicit x / Cancel controls wired via data-addbox-close. The disclosures stay fully usable without the script (#545). - The JSON pane's header bunched Collapse all / Expand all / Edit raw together on the right; the fold controls now sit next to the JSON heading and Edit raw stays alone at the end (#548). Closes #544. Closes #545. Closes #548.
…ecents cap The #542 investigation's findings, applied: recentSearches is bounded at ten on write, but the dropdown also renders every saved query in the same panel with no scroll container, so a long collection pushed it off screen. The list region now scrolls at min(420px, 60vh) with the heading fixed, and recentSearches() re-asserts the cap on read as a belt against oversized documents written by other clients. Closes #542.
11 tasks
…sistance
Two more items from the UI review pass:
- Creating a resource with required elements dumped bare, duplicated
lines ('priority: priority is required') under the Guided form. The
unanchored issues now dedupe, drop the doubled element name (the
validator's message already opens with it), sort deterministically,
and render in a bounded, titled alert panel that tells the user the
fix is adding the elements (#546).
- FHIR temporal primitives (date, dateTime, time, instant) were bare
text boxes. Each now carries a format placeholder, an input pattern
driving live native :invalid styling, and a spec-accurate title hint
-- a malformed birthDate shows red before any server round trip, and
partial dates (YYYY, YYYY-MM) stay legal, which is why these are
patterns rather than native date pickers (#549).
Closes #546. Closes #549.
angela-helios
changed the title
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
…re-renders The editor re-renders its whole body on every mutation by design (the document is the state; validation is server-side). What #547 calls out is everything that re-render destroyed: the focused field and caret, any open add-picker with its filter text, and the tree scroll — plus the empty first-run tree that gave a new resource nothing to act on. Four changes, applied to both editor.js and the resources modal's copy of the cycle: - A blur with an unchanged value no longer round-trips at all: tabbing through fields stops re-rendering the panel. - The swap captures the UI state at response time (where the user is NOW, not where they were when the request left) and restores it after: focus + caret by data-set path, open pickers with filter text re-applied, tree scrollTop. - A mutation that creates a node reports its path (apply() now returns the created Path; the fragment carries it as data-focus on #editor-form) and the caret lands in the new field, selected, ready to type. The picker stays open, so adding five elements in a row is five clicks and five values. - A document with nothing beyond resourceType opens the root add-picker by itself. Closes #547.
angela-helios
changed the title
…keys test The bulk-submit JWKS work brought in the pure-Rust rsa crate for RS384 public-JWK derivation, tripping the Security Audit on every PR: RUSTSEC-2023-0071 (Marvin Attack timing sidechannel) has no fixed release, and jwe.rs already rejects RSA-OAEP for exactly this reason. The JWKS handler's own doc promises an empty key set for non-ES384 algorithms -- the code now honors it. Both copies of the derivation (rest's derive_public_jwk, ui's signing_kid) return None for RS384 with the rationale in place; an RS384 deployment registers its key out-of-band, the assertion goes out without a kid, and the rsa crate leaves the lockfile. Also updates the UI keys test the redirect change left behind: the route permanently redirects to /.well-known/bulk-submit-jwks.json now, and the old 404/200 assertions were failing Test Rust on every PR's merge ref.
Contributor
Author
|
Brought this branch up to date with main (which now carries #539) and cherry-picked the #560 fix onto it, so the merge ref is green regardless of merge order: the rsa crate is out of the lock here too, and the keys test asserts the redirect contract. If #560 merges first, the shared changes collapse to a no-op on this side. |
…ns idempotent Both fallouts of the root add-picker auto-opening on an empty document (#547): - axe can now see the value[x] choice selects that used to hide inside the collapsed details, and they never had an accessible name — each carries an aria-label built from the declarer and the pick-type message. - The editor-controls specs clicked the panel summary to open it, which now toggles an already-open picker closed and strands the fill behind an invisible filter. The page object gains openAddPanel() (click only when closed) and the value[x] spec checks the open attribute before toggling. Verified locally: the affected editor-controls and a11y editor specs pass against the rebuilt binary (9/9).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #542. Closes #544. Closes #545. Closes #546. Closes #547. Closes #548. Closes #549.
Three of the quick items from the UI review pass, batched because they share the chrome:
#544 — tenant picker gating. The sidebar picker rendered on every install, offering exactly one choice on single-tenant servers. It now renders only when the effective tenant differs from the default or the registry knows a second tenant. The check rides the existing
resolve_prefsmiddleware (one indexedlist_tenantsread per page load — the same cost class as the settings read it already performs), flows throughRequestTenant.multi→Status::show_tenant_picker(), and the layout gates on it. Tested both directions: hidden on a fresh single-tenant install, appears the moment a second tenant is provisioned through the page's own form.#545 — Add-tenant panel escape hatches. The panel had a submit button and no way out. A small
addbox.jsasset (defer-loaded, ~30 lines, no framework) adds three: Esc, click outside, and explicit × / Cancel controls wired viadata-addbox-close. The same affordances land on the bulk-import dialogs, which had the same gap. The<details>disclosures remain fully usable with JavaScript disabled — the script only adds ways out, per the crate's no-JS-first rule.#548 — JSON pane header layout. Collapse all / Expand all now sit next to the "JSON" heading (a
card-head__tools--startgroup withmargin-right: auto); Edit raw stays alone on the right.Full helios-ui suite green (13 tenants tests incl. the two new behavior tests, i18n parity ×3 with the new
ui-cancel/ui-closekeys), clippy clean,--features uibuilds.Added after opening: the #542 investigation landed here too — the Recent/Saved dropdown now scrolls at min(420px, 60vh) and the recents cap is re-asserted on read; full findings on the issue.
Second batch (same review pass):
#546 — orphan issues panel. Creating a resource with required elements dumped bare, duplicated lines ("priority: priority is required") under the Guided form. Unanchored issues now dedupe, drop the doubled element name (the validator's message already opens with it), and render in a bounded, titled alert panel. The deeper "give each missing required element an actual field" belongs with the #547 authoring-UX work.
#549 — temporal input assistance.
date/dateTime/time/instantinputs carry a format placeholder, a spec-shapedpatterndriving live native:invalidstyling (red before any server round trip), and a hint title. Patterns rather than native date pickers on purpose: FHIR's partial dates (2024,2024-05) are legal and native pickers can't express them.Third batch — #547, guided authoring. The editor's full-body re-render is by design (the document is the state, validation server-side); what it destroyed was the user's place. Now: a blur with an unchanged value doesn't round-trip at all (tabbing stops re-rendering); the swap captures UI state at response time and restores focus + caret, open add-pickers with their filter text, and tree scroll; a mutation that creates a node reports its path (
data-focuson the fragment) and the caret lands in the new field selected and ready to type, with the picker left open so multi-add flows; and an empty document opens the root picker by itself. Applied to both editor.js and the resources modal's copy of the cycle. This also delivers the deeper half of #546: required-element issues now have an immediate path to a field (auto-open picker → click → focused input).