`Development`: Use real buttons and links instead of click-handler divs by az108 · Pull Request #2529 · ls1intum/doc-apply

az108 · 2026-05-14T22:16:40Z

Checklist

General

Language: I followed the guidelines for inclusive, diversity-sensitive, and appreciative language.
I chose a title conforming to the naming conventions for pull requests.

Client

Important: I implemented the changes with a very good performance, prevented too many (unnecessary) REST calls and made sure the UI is responsive, even with large data (e.g. using paging).
I strictly followed the principle of data economy for all client-server REST calls.
I strictly followed the client coding and design guidelines.
I added multiple integration tests (Vitest) related to the features (with a high test coverage), while following the client test guidelines.
I documented the TypeScript code using JSDoc style.

Motivation and Context

Closes #2395.

Across the app many templates implemented the same accessibility pattern by hand on non-semantic elements (<div>, <p>, <article>): role="button" + tabindex="0" + (click) + (keydown.enter), sometimes with an extra (keydown.space). That duplication made behaviour inconsistent — one row also handled Space, the next didn't; <a> tags without href had to fake Enter activation themselves — and it offers a worse experience than a real <button> or <a>: no open-in-new-tab for links, no native keyboard contract, no automatic focus ring.

Description

The audit found 23 occurrences across 16 files, in three buckets.

Button-like (10) — now <button type="button">, with a small .btn-bare class in global.scss that strips the button chrome so the existing card and row styling survives. Affected: sidebar-button, image-upload-button, upload-button, rating segments, string-input helper-text link, selectable-slot-card, interview-process-card, document-dialog rows, the profile-picture-settings avatar trigger, and the login / registration switch-view links, which were <a> without href and so never links at all.

Link-like (4) — now <a [routerLink]>. Affected: the footer's imprint, privacy and about-us links, whose navigation methods on FooterComponent are gone, and job-card, where an onViewDetails / onKeyDown pair became [routerLink]="detailLink()".

Interactive content inside (5) — kept as <div> with a new [jhiClickable] directive that owns the role, the tabindex and Enter/Space activation. These are the cases where the element contains another interactive element, which the spec forbids inside a <button> or <a>: interviewee-card, interviewee-section, assign-applicant-modal, upcoming-interview-card, and the image card in job-creation-form.

click-events-have-key-events was given ignoreWithDirectives: ['jhiClickable'], so the directive is a first-class way to satisfy the rule instead of inline keyboard handlers.

What changed during review

Testing this turned up a set of keyboard problems, some of which predate the PR:

The directive was swallowing keys from nested elements. Key events bubble, so Enter on a nested delete button ran the wrapper's handler, which cancelled the button's own activation and clicked the wrapper instead — tabbing to the trash icon on an image card selected the image rather than deleting it. It now ignores key presses that did not happen on the host, which is the very case it exists for.
The compliance banner needed its delegation back. The link inside that translated message is an anchor without an href, so it never fires a click of its own; the wrapper used to catch the bubbled key press and recognise the link by its class. Going through the directive replaced that with a click on the wrapper, so the class never matched. It is a role="presentation" delegator again, and no longer takes a tab stop of its own.
Confirmation dialogs left focus behind the mask. PrimeNG focuses the first control of its own header, content or footer, and a headless dialog renders none of those, so the dialog could not be reached by keyboard at all. It now moves focus itself and hands it back to whatever opened it on close. Ours is the only headless dialog in the app; every other popup already behaved.
The delete button on an image card is invisible until the card is hovered. opacity-0 still leaves an element focusable, so tabbing landed on a control nobody could see. It now appears on focus as well.
Both candidate lists cost one tab stop per person, which is a long way round when a search returns twenty-five professors — and the add-members rows cost two each, the row plus the checkbox inside it. Both are now listboxes: one tab stop, arrows to move, Home and End for the ends, Enter or Space to pick, Tab to leave. Rows are options that report whether they are selected, and the add-members checkbox now only shows the state its row already announces.
Layout fixes: the sidebar button stretches so its highlight covers the whole row; the upload tile fills its grid cell again, having collapsed to the size of its icon when a block div became an inline-block button; and .btn-bare is down to the two declarations Tailwind does not already reset, inside @layer components so utilities still win.

job-card no longer activates on Space. That is deliberate: it is a real link now, and links activate on Enter while Space scrolls the page.

Steps for Testing

Prerequisites: log in and visit each affected surface.

Mouse, unchanged behaviour

Sidebar items, footer links, login and register switch links, upload buttons, rating segments, the string-input helper link.
Job overview cards — and note that right-click, open in new tab now works, which it could not before.
Job creation image picker, document dialog list, profile picture editor, interview cards, slot cards, interviewee rows, filter multiselect.

Keyboard

Tab through each surface: everything clickable is still reachable, Enter activates, Space activates buttons but not links.
On an image card, tab to the trash icon. It should become visible on focus, and Enter should open the delete dialog rather than selecting the image.
With that dialog open, Tab should move between Cancel and the confirm button. Close it and focus should land back on the trash icon, not at the top of the page.
Do the same for deleting an uploaded file in the upload component.
In the job creation compliance banner, tab to the review link inside the message and press Enter. It should jump to the first step.
Open the research group creation form as an admin, search for professors, and tab into the results. The list should be a single tab stop: arrows move between candidates, Home and End jump to the ends, Enter picks one, and Tab continues to the next field.
Repeat in the Add members dialog. Each person should be one stop rather than two, and Space should toggle selection.
Search again for something with fewer results while further down the list, then tab back in — the list should still be reachable.

Review Progress

Code Review

Code Review 1

Manual Tests

Mouse activation is unchanged everywhere
Enter activates; Space activates buttons but not links
Nested buttons inside clickable wrappers act for themselves
Confirmation dialogs take focus on open and give it back on close
The trash icon on an image card is visible when focused
Both candidate lists are one tab stop and move under the arrow keys
No visual regressions on cards, rows or icon buttons

Test Coverage

Client

Class/File	Line Coverage	Lines	Expects	Ratio
interviewee-card.component.ts	0.00%	61	11	18.0
interviewee-section.component.ts	0.00%	360	41	11.4
assign-applicant-modal.component.ts	0.00%	142	?	?
upcoming-interview-card.component.ts	0.00%	43	9	20.9
job-creation-form.component.ts	0.00%	1256	96	7.6
job-card.component.ts	100.00%	78	8	10.3
footer.component.ts	0.00%	28	9	32.1
checkbox.component.ts	0.00%	31	?	?
confirm-dialog.ts	95.91%	83	11	13.3
image-upload-button.component.ts	0.00%	121	8	6.6
sidebar-button.component.ts	95.23%	37	7	18.9
research-group-creation-form.component.ts	0.00%	396	?	?
clickable.directive.ts	100.00%	33	12	36.4
listbox.util.ts	100.00%	19	7	36.8
research-group-add-members.component.ts	0.00%	201	40	19.9
jobs-preview-section.component.ts	100.00%	51	12	23.5

Last updated: 2026-07-28 14:53:04 UTC

The codebase had dozens of `<div>`/`<p>`/`<article>` elements made clickable by hand with `role=button` + `tabindex=0` + `(click)` + `(keydown.enter)`. This converted every case that can be expressed semantically to a real `<button type="button">` (with a small `.btn-bare` reset class), real `<a [routerLink]>`, or — where an interactive descendant blocks both — a new `jhiClickable` directive that owns the `role`/`tabindex` host attributes and the Enter/Space activation. Dead `stopPropagation`-only wrappers around delete-button overlays were removed; one remaining propagation guard keeps its no-role/no-tabindex form so the focus ring is not stolen from the inner button. Closes #2395 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

codacy-production · 2026-05-14T22:18:01Z

Not up to standards ⛔

🔴 Issues 3 high

Alerts:
⚠ 3 issues (≤ 0 issues of at least minor severity)

Results:
3 new issues

Category Results

Security 3 high

View in Codacy

🟢 Metrics 49 complexity

Metric Results

Complexity 49

View in Codacy

_{NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer}
_{TIP This summary will be updated as you push new changes.}

github-actions · 2026-05-14T22:18:21Z

📊 Client Test Coverage Too Low

🔍 View coverage locally:

npm run test:ci
open build/test-results/vitest/coverage/index.html

🌐 View coverage from GitHub:
Download the "coverage-report-client" artifact from this workflow run.

…ctive-elements # Conflicts: # src/main/webapp/app/shared/components/atoms/upload-button/upload-button.component.html # src/main/webapp/app/shared/components/molecules/document-dialog/document-dialog.html

…ctive-elements # Conflicts: # src/main/webapp/app/layouts/footer/footer.component.html # src/main/webapp/app/layouts/footer/footer.component.ts # src/main/webapp/app/shared/components/atoms/upload-button/upload-button.component.html

github-actions · 2026-05-29T13:02:46Z