IS-11218 Render BankID QR accessibility messages as collapsible sections by aleixsuau · Pull Request #253 · curityio/ui-kit

aleixsuau · 2026-07-02T13:37:13Z

Jira: https://curity.atlassian.net/browse/IS-11218

Frontend counterpart to the BankID server work (server PR #10231) that adds QR-code accessibility messages under metadata.viewData.messages.

What

Renders the BankID QR accessibility messages in BankIdViewNameBuiltInUI as two collapsible <details> sections — "Help with scanning the QR code" (instruction.*) and "If you are using a screen reader" (screen-reader.*) — mirroring the classic Velocity layout.

Test instructions

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Copilot

Pull request overview

Adds frontend support for BankID QR-code accessibility messages delivered via metadata.viewData.messages, rendering them in the built-in BankID UI as two collapsible <details> sections to match the classic Velocity layout.

Changes:

Introduces HaapiStepperBankIdQrAccessibilityMessages to render “instruction.” and “screen-reader.” message groups as collapsible sections.
Adds message-key normalization via getQrViewDataMessages to resolve keys by their .view.qr.<suffix> tail.
Extends HAAPI step typings with metadata.viewData.messages, adds styling and documents the new CSS hook.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
src/haapi-react-sdk/haapi-stepper/util/qr-view-data-messages.ts	Adds normalization helper for `.view.qr.`-scoped message keys.
src/haapi-react-sdk/haapi-stepper/README.md	Documents the new `.haapi-stepper-bankid-qr-accessibility` CSS hook.
src/haapi-react-sdk/haapi-stepper/feature/viewnames/index.ts	Exports the new BankID QR accessibility messages component.
src/haapi-react-sdk/haapi-stepper/feature/viewnames/HaapiStepperBankIdQrAccessibilityMessages.tsx	Implements the collapsible accessibility sections for BankID QR messages.
src/haapi-react-sdk/haapi-stepper/feature/viewnames/HaapiStepperBankIdQrAccessibilityMessages.spec.tsx	Adds test coverage for rendering and toggle behavior.
src/haapi-react-sdk/haapi-stepper/feature/viewnames/BankIdViewNameBuiltInUI.tsx	Wires the new component into the built-in BankID view.
src/haapi-react-sdk/haapi-stepper/data-access/types/haapi-step.types.ts	Adds `HaapiViewData` and types `metadata.viewData` (incl. `messages`).
src/haapi-react-app/src/shared/util/css/styles.css	Styles the new container and `<details>` spacing/behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

…atting layer

luisgoncalves · 2026-07-07T15:12:04Z

I tested locally and overall looks nice! One thing I noticed is that the progress bar is displayed bellow the new collapsible sections, which becomes quite off, name when the panels are expanded. I think we need to fix this.

As mentioned in other comments, I also think that it would be better to contain the processing of this stuff in the BankID-specific view/code.

For these reasons, I'm not approving yet.

luisgoncalves · 2026-07-07T14:51:41Z

+     * during step-data formatting. Present only on the QR-code link when the server supplied a
+     * complete section.
+     */
+    qrCodeAccessibility?: HaapiStepperQrCodeAccessibility;


I see what you tried to do here, but conceptually, it's kind of weird that a link has "accessibility details".

I think this is too specific to BankID to be defined here. Also, because It means the formatting layer:

Assumes some structure about the messages (see comments there) which I think are a bit of a stretch.

Assumes the messages apply to a certain link, but they don't. They are something for the whole view.

I think we should not model such a strong relation now, because we don't have other use cases (YAGNI and generalizing too early). We are defining a bigger API surface that we need to support later on. If this is done in the BankID "built in UI", is much more contained to start with.

I see what you tried to do here, but conceptually, it's kind of weird that a link has "accessibility details".

I think this is too specific to BankID to be defined here. Also, because It means the formatting layer:

Assumes some structure about the messages (see comments there) which I think are a bit of a stretch.

Assumes the messages apply to a certain link, but they don't. They are something for the whole view.

I think we should not model such a strong relation now, because we don't have other use cases (YAGNI and generalizing too early). We are defining a bigger API surface that we need to support later on. If this is done in the BankID "built in UI", is much more contained to start with.

I see your point 👀 🤔 . I think this follows a design decision made at the beginning of the project; the Data Layer.

Due to some limitations, we opted from the beginning for a Data Layer to normalize and enrich the HAAPI responses so they can be consumed more easily by the FE. This includes renaming and relocating data, retyping it (HaapiStepper* typings), and adding new data (id, type, subtype) by extending (not replacing) the original interfaces. These are represented by the HaapiStepperDataHelpers at the step level, and by HaapiStepperDataHelpersDetails at the element detail (Action (form, selector, and client operation) Message, and Link).

This decision aligns with another FE design principle, keep UI focused on supporting the view and interaction, encapsulating all the business logic (data management, formatting, state management...) in services/providers/hooks.

This pattern is already used for generic element properties like id o type, but also for specific ones like maxWaitTime, which are step-scoped (step.properties) and relocated onto the polling client-operation action they concern.

It seems the question would be, is this data related to the step or to the element (e.g. link)?

The accesibility texts describe how to interact with the QR link:

From the perspective of a generic library for rendering steps, which are a composition of Actions, Links and Messages, I also think that data belongs to the link. When do we need those accessibility texts? I think when a QR link is present in the screen. Should a link render interceptor have access to that data? I think so.

That said, I agree on the mixed concerns between the Data Layer and the UI and refactored it following your recommendations (see last commit). The Data Layer no longer touches the keys at all; and the accessibility messages' logic moved into the BankID built-in UI, which now looks up messages by their exact full keys (authenticator.bankid.launch.view.qr.*). WDYT?

Thanks for the recap on the concerns of the "data layer". Some of the things it does are generic enough, like the polling properties you mentioned. These are part of the schema of all polling steps, regardless of the view/response where they are included.

That's not the case for accessibility messages being discussed. In the example at hands the messages describe the QR code link, but, as I said, I don't think we have enough information to generalize this. Steps could have multiple images and a single helper section; also, the structure of this helper section is highly step-dependent so far.

Thanks for considering and for the follow up. I'll check the latest version.

Sorry, but I still think links shouldn't have "qr code messages". It's too broad and early generalization, also because assuming that .view.qr. being present in the messages means something feels quite random - and it could be that this message is for something else (keep in mind that the server is extensible and custom plugins could produce any HAAPI response with view data messages).

Also note that, when the team discussed this from the server perspective, it was decided to add messages to step "view data", and not to add a new property on links to convey these messages. I think this shows how we thought about it, conceptually: a step concern, not a link concern.

luisgoncalves · 2026-07-07T14:59:03Z

+ * after `.view.qr.` (e.g. `instruction.heading`, `screen-reader.step4.2.1`). Keying by the suffix
+ * keeps this independent of the message-key prefix; keys without the marker are ignored.
+ */
+function stripQrCodeViewPrefix(messages?: Record<string, string>): Record<string, string> {


This is a bit of a stretch. The server returns messages with full keys to be precise and future proof for other added messages (e.g. partially matching). View data messages are specific to a certain representation (i.e. to a certain viewName).

We don't know anything about other possible use cases. Why would we assume this message structure could be reused/similar in other cases? I don't see the need for this assumption and generalization.

The logic to check for message presence and building typed sections is great (and extracting it to a dedicated file), but I suggest doing this in the BankID-specific namespace.

We can always generalize later, but don't compromise for now.

(you know I'm more conservative when it comes to this kind of thing 😃 but I think I presented good arguments)

Please refer to my previous comment 🙏

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

vahag-curity

Review notes from a read-through of the diff, the surrounding formatter/view code, and the server bankid/launch.properties bundles. Nice, well-tested change overall — clean separation between the generic selection in the formatter and the BankID-specific interpretation in the component, and I verified the 17 hardcoded keys match the server bundle exactly and are present in all shipped locales (sv/pt/pt-pt). A few inline notes below; only the first is something I'd want addressed before merge.

checked: no XSS (all copy rendered as escaped React text), metadata? is on HaapiBaseStep so the optional chain is sound for every step type, and ::details-content is a safe progressive-enhancement reset.

vahag-curity · 2026-07-09T11:09:46Z

  };
 }

+function addLinkDataHelpers(


There's no format-next-step-data.spec.ts — this seam is the highest-blast-radius change in the PR since addLinkDataHelpers runs for every link of every step. The leaf selector and the component are both well covered, but nothing asserts the wiring: that qrCodeMessages lands on the QR link and not on sibling non-QR links, and that steps without metadata/viewData still format unchanged. Could we add a focused test here? Happy to draft it if useful.

vahag-curity · 2026-07-09T11:09:46Z

+  step: HaapiActionStep | HaapiCompletedStep | HaapiStepperStep
+): HaapiStepperLink {
+  const linkWithDataHelpers = getElementWithDataHelpers(link);
+  const qrCodeMessages = getQrCodeViewDataMessages(step.metadata?.viewData?.messages);


Minor: this is step-scoped but recomputed for every link (it re-Object.entries().filter()s the whole message map each call), while the result is only ever used on the single QR link. Could hoist it out of addLinkDataHelpers and compute once per step. Negligible in practice (few links), just trivially avoidable and a bit clearer.

vahag-curity · 2026-07-09T11:09:46Z

+      return typeof value === 'string' && value.length > 0;
+    });
+
+  const showInstruction = has(


All-or-nothing gate: a section renders only if all its keys resolve non-empty. That's the right call for the nested screen-reader tree (avoids orphaned sub-items), and I confirmed all shipped locales are complete. The edge case worth a conscious decision: customers can override message bundles, and if an override drops even one sub-key (e.g. screen-reader.step4.2.2), the entire accessibility section silently disappears — a sharp failure mode for an accessibility feature specifically. Defensible trade-off, but maybe worth a short comment noting it's intentional.

This was discussed and agreed on (but can be discussed again, ofc).

vahag-curity · 2026-07-09T11:09:46Z

+  }
+
+  return (
+    <div className="haapi-stepper-bankid-qr-code-accessibility" data-testid="bankid-qr-code-accessibility">


Nit: this data-testid="bankid-qr-code-accessibility" on the container isn't referenced by any test (the specs use the per-section ids). Harmless, but could drop it or add an assertion.

vahag-curity · 2026-07-09T11:09:47Z

 }

 .haapi-stepper-polling-progress {
+  @extend .mt2;


This @extend .mt2 isn't scoped to the BankID accessibility context, so it adds top margin to .haapi-stepper-polling-progress everywhere it renders, not just here. Just confirming that spacing change is intended for all polling views and not only the BankID flow.

Yes, intended. .haapi-stepper-polling-progress is rendered by the shared HaapiStepperClientOperationUI for any polling client operation, so the mt2 gives the progress bar consistent top spacing from whatever precedes it in the actions area .

…uilt-in UI. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

luisgoncalves · 2026-07-09T15:09:40Z

+      return typeof value === 'string' && value.length > 0;
+    });
+
+  const showInstruction = has(


This was discussed and agreed on (but can be discussed again, ofc).

luisgoncalves · 2026-07-09T15:15:46Z

      {actionsElement}
-      {nonQrLinks.length > 0 && getLinksElement(props, nonQrLinks, linkRenderInterceptor)}
+      {nonQrCodeLinks.length > 0 && getLinksElement(props, nonQrCodeLinks, linkRenderInterceptor)}
    </>


The progress bar for the "authentication time" is now displayed below the instructions, which is not aligned with the reference UI in Velocity. I think we need to adjust this.

The current implementation is also not working well with the different "modes" on the authenticator when "Backend Return URL" is enabled. This is a recent feature and we didn't account for it.

In this case, it's possible to have the QR code without the client-operation, or the client-operation without the QR code. I suggest we address all these details together separately, but we need to get to them.

IS-11218 Render BankID QR accessibility messages as collapsible sections

e2a21a4

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

aleixsuau requested review from Copilot, luisgoncalves, urre and vahag-curity July 2, 2026 13:43

Copilot started reviewing on behalf of aleixsuau July 2, 2026 13:45 View session

Copilot AI reviewed Jul 2, 2026

View reviewed changes

Comment thread src/haapi-react-sdk/haapi-stepper/data-access/types/haapi-step.types.ts

aleixsuau marked this pull request as ready for review July 6, 2026 11:57

IS-11218: refactor to locate accessibilitydata formatting in the form…

d232f2f

…atting layer

luisgoncalves reviewed Jul 7, 2026

View reviewed changes

aleixsuau and others added 3 commits July 8, 2026 08:29

IS-11218 Format QR accessibility component with Prettier

770fdbb

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

IS-11218: move bankid ui-specific logic to the built-in viewname ui

2f12a3c

IS-11218 Fix doc comment punctuation.

3c7c2b9

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

vahag-curity reviewed Jul 9, 2026

View reviewed changes

IS-11218 Move QR accessibility view-data formatting into the BankID b…

495a462

…uilt-in UI. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

luisgoncalves approved these changes Jul 9, 2026

View reviewed changes

Uh oh!

Conversation

aleixsuau commented Jul 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What

Test instructions

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

luisgoncalves commented Jul 7, 2026

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

luisgoncalves Jul 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

luisgoncalves Jul 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vahag-curity left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

aleixsuau commented Jul 2, 2026 •

edited

Loading

luisgoncalves Jul 8, 2026 •

edited

Loading

luisgoncalves Jul 7, 2026 •

edited

Loading