ZCU-PUB/Add accessible labels to the restricted-bitstream lock icon - #1390
Merged
milanmajchrak merged 5 commits intoJul 21, 2026
Merged
Conversation
- Make the space between the translated "Download" prefix and the bitstream name explicit in the template instead of relying on a trailing space in the en.json5 source string (cs.json5 had none, producing "Stáhnoutname.pdf" with no separator). - aria-label on a plain <span> is not reliably exposed by assistive tech; switch to the codebase's existing sr-only pattern (see view-mode-switch.component.html) and mark the decorative lock icon aria-hidden. - Add spec coverage asserting the download link's aria-label and the restricted-state sr-only text/aria-hidden icon.
Defense-in-depth on top of the explicit-space fix: trims the translated prefix and the final concatenated string, so the label stays correctly spaced even if a translation (any of the ~30 locale files, not just cs) carries stray leading/trailing whitespace.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
MatusBeke
added a commit
that referenced
this pull request
Jul 20, 2026
Fixes dataquest-dev/dspace-customers#823 (bitstream portion). Ports the real vanilla DSpace feature that already exists on this fork's DSpace 9.x lines (e.g. customer/mendelu): a unified AccessStatusBadgeComponent that renders an access-status badge for either an Item or a Bitstream, instead of a separate embargo-only component (an earlier draft of this PR introduced a standalone EmbargoBadgeComponent copied from a since-abandoned upstream design (PR dataquest-dev/dspace-angular#3882 / commit 5f83139); that component has no live upstream reference on 9.3/main/mendelu and has been removed in favor of this unified approach). Change set: - Bitstream model gains an `accessStatus` HAL link (mirrors the existing thumbnail/format link shape). - AccessStatusObject (both the canonical copy and a pre-existing duplicate under object-list/) gains `embargoDate`. - AccessStatusBadgeComponent's `object` input widens from `Item` to `Item | Bitstream`; it now resolves the accessStatus link lazily via LinkService.resolveLink(..., followLink('accessStatus', { isOptional: true })) instead of eagerly reading `_links.accessStatus.href` directly. isOptional: true is required (unlike upstream, which never needs it because its backend always has the link already) because this backend doesn't expose the bitstream-level link yet - without it, resolveLink() throws for every bitstream instead of failing closed. - file-download-link.component.html renders <ds-themed-access-status-badge [object]="bitstream"> next to each file, gated by the existing item.bitstream.showAccessStatuses flag. - That flag now defaults to false (matching upstream/mendelu) instead of true - flip it once the backend companion PR (DSpace#1377) is deployed, not before. - i18n copy aligned with vanilla/mendelu: "Embargo until {{ date }}" (en) / "Embargo do {{ date }}" (cs). - Removed the unused findBitstreamAccessStatusFor() and the deleted EmbargoBadgeComponent/ThemedEmbargoBadgeComponent + their shared .module.ts registration. The accessibility fix (aria-label/role/sr-only on the lock icon) that was previously bundled into this branch has been split out to its own PR (#1390) for independent review - this branch was rebuilt from scratch off customer/zcu-pub to drop those commits and keep this PR scoped to the embargo-badge feature only. Test evidence: npx ng lint --quiet -> All files pass linting. npx ng build --configuration production -> build succeeded, no budget warnings npx ng test (access-status-badge + file-download-link + thumbnail specs) -> 39/39 SUCCESS, including a regression test that constructs a real Bitstream lacking _links.accessStatus and asserts ngOnInit doesn't throw and the file list renders without a badge. Manual: local Docker stack running the CURRENT (pre-#1377) zcu-pub backend image, showAccessStatuses temporarily forced true - an embargoed item's file list rendered normally, zero console errors, and no accessStatus network request was even attempted (LinkService detects the missing _links key up front).
An element's aria-label overrides its accessible-name computation entirely, so the nested sr-only "Restricted bitstream" text was never actually announced by assistive tech - the lock icon's aria-hidden state was correct, but the restriction reason was silently dropped. Fixed by composing a single aria-label (via the new getDownloadLinkAriaLabel() method) that includes the restricted state when applicable, instead of relying on descendant sr-only content. Also evaluated Copilot's *ngVar suggestion to avoid the two separate canDownload$ | async subscriptions (aria-label + the lock icon's *ngIf) - reverted it after it broke the routerLink binding in tests: *ngVar destroys and recreates its entire templated content (including the anchor and its RouterLink) on every emission, which would cause the download link to flicker/reset once the authorization check resolves. Kept the two independent `| async` pipes instead, consistent with how bitstreamPath$ is already used twice in this same template.
milanmajchrak
pushed a commit
that referenced
this pull request
Jul 21, 2026
Fixes dataquest-dev/dspace-customers#823 (bitstream portion). Ports the real vanilla DSpace feature that already exists on this fork's DSpace 9.x lines (e.g. customer/mendelu): a unified AccessStatusBadgeComponent that renders an access-status badge for either an Item or a Bitstream, instead of a separate embargo-only component (an earlier draft of this PR introduced a standalone EmbargoBadgeComponent copied from a since-abandoned upstream design (PR dataquest-dev/dspace-angular#3882 / commit 5f83139); that component has no live upstream reference on 9.3/main/mendelu and has been removed in favor of this unified approach). Change set: - Bitstream model gains an `accessStatus` HAL link (mirrors the existing thumbnail/format link shape). - AccessStatusObject (both the canonical copy and a pre-existing duplicate under object-list/) gains `embargoDate`. - AccessStatusBadgeComponent's `object` input widens from `Item` to `Item | Bitstream`; it now resolves the accessStatus link lazily via LinkService.resolveLink(..., followLink('accessStatus', { isOptional: true })) instead of eagerly reading `_links.accessStatus.href` directly. isOptional: true is required (unlike upstream, which never needs it because its backend always has the link already) because this backend doesn't expose the bitstream-level link yet - without it, resolveLink() throws for every bitstream instead of failing closed. - file-download-link.component.html renders <ds-themed-access-status-badge [object]="bitstream"> next to each file, gated by the existing item.bitstream.showAccessStatuses flag. - That flag now defaults to false (matching upstream/mendelu) instead of true - flip it once the backend companion PR (DSpace#1377) is deployed, not before. - i18n copy aligned with vanilla/mendelu: "Embargo until {{ date }}" (en) / "Embargo do {{ date }}" (cs). - Removed the unused findBitstreamAccessStatusFor() and the deleted EmbargoBadgeComponent/ThemedEmbargoBadgeComponent + their shared .module.ts registration. The accessibility fix (aria-label/role/sr-only on the lock icon) that was previously bundled into this branch has been split out to its own PR (#1390) for independent review - this branch was rebuilt from scratch off customer/zcu-pub to drop those commits and keep this PR scoped to the embargo-badge feature only. Test evidence: npx ng lint --quiet -> All files pass linting. npx ng build --configuration production -> build succeeded, no budget warnings npx ng test (access-status-badge + file-download-link + thumbnail specs) -> 39/39 SUCCESS, including a regression test that constructs a real Bitstream lacking _links.accessStatus and asserts ngOnInit doesn't throw and the file list renders without a badge. Manual: local Docker stack running the CURRENT (pre-#1377) zcu-pub backend image, showAccessStatuses temporarily forced true - an embargoed item's file list rendered normally, zero console errors, and no accessStatus network request was even attempted (LinkService detects the missing _links key up front).
milanmajchrak
pushed a commit
that referenced
this pull request
Jul 23, 2026
…Statuses) (#1403) * ZCU-PUB/Enable per-bitstream embargo badge (item.bitstream.showAccessStatuses) The embargo-date badge (dspace-customers#823, FE #1378/#1390) is gated behind item.bitstream.showAccessStatuses, which defaults to false in default-app-config.ts. Set it to true in config.yml so the badge renders on redeploy. Safe now that the backend accessStatus endpoint is deployed for zcu-pub (DSpace#1377); the frontend fails closed without it. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Simplify embargo badge comment --------- Co-authored-by: Matus Kasak <matus.kasak@dataquest.sk> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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
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.
Problem
The bitstream download link in the Item View gives no indication why a file is locked — the lock icon next to a restricted/embargoed bitstream has no accessible label, and the download link itself has no descriptive label either.
Split out of #1378 for independent, focused review (that PR now covers the embargo-date badge feature only — see its updated description).
Root cause
FileDownloadLinkComponentoncustomer/zcu-pubpredates an accessibility fix that already exists ondtq-dev(the integration/vanilla branch): the lock icon and the download link lackaria-labels that surface the embargo/restricted state.Change set
file-download-link.component.html—aria-label/role/tabindexon the download<a>; the lock icon isaria-hidden="true"with an adjacentsr-only"Restricted bitstream" text (matches thesr-onlypattern already used elsewhere in this codebase, e.g.view-mode-switch.component.html).file-download-link.component.ts— injectDSONameServicefor the new aria-label.file-download-link.component.spec.ts—TranslateModule.forRoot()+ coverage for both new labels.en.json5/cs.json5—file-download-link.download/file-download-link.restrictedkeys.This is the same content originally reviewed and approved via Copilot on #1378 (all 3 review threads there were resolved) — cherry-picked cleanly onto a fresh branch off
customer/zcu-pubwith no changes.Test evidence
Risk & rollback
Low risk: additive
aria-label/role/tabindexattributes and two new i18n keys on a single shared component. No behavioral/routing changes. Revert is a straightgit revert.Notes / assumptions
Companion PR #1378 (embargo-date badge) was rebased to drop these same commits from its history, to keep the two concerns reviewable independently — see that PR's updated description for why.