UoE/WP2-2026-07-09 bugfixes - #6
Merged
dspeed2 merged 22 commits intoJul 9, 2026
Merged
Conversation
Replace the per-file "Format" line in the full item view file listing with a "File checksum" line (algorithm + value). Add the checkSum field to the Bitstream model so the REST value deserializes, and add the item.page.filesection.checksum i18n key. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Wrap the full item view checksum dt/dd in *ngIf="file?.checkSum" and render only the checksum value (drop the algorithm prefix), so no stray colon shows when a bitstream has no checksum. Propagate item.page.filesection.checksum to all locale files with an English placeholder. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Match DSpace/dspace-angular@main (commit 4f72074): use the exported ChecksumInfo interface and `checkSum: ChecksumInfo` positioned after `description`, replacing the inline anonymous type, to minimize conflicts on a future DSpace 9 migration. Display and i18n key are unchanged (upstream renders no checksum row in the full item view). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
UoE instance only ships English; other locale .json5 files aren't maintained here. Revert the checksum key addition to all 27 non-en locales, keep it in en.json5 only. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
file is guaranteed by *ngFor and checkSum is truthy inside the *ngIf guard, so use plain file.checkSum / file.checkSum.value to match the surrounding template style. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…chMap -> concatMap) Backport of the save-effect change from upstream PR DSpace#5145 (commit a3c14c5, first released in DSpace 10.0; not present in 8.x/9.x). The four submission save effects used switchMap. When two save actions of the same type overlap (common during editing: unguarded section/upload/save-for-later saves, save-on-change on validation errors, or an enabled autosave timer), switchMap cancels the in-flight save's inner observable AFTER it dispatched StartTransactionPatchOperationsAction (commitPending=true) but BEFORE its response handler runs. Commit/Rollback are dispatched only inside that cancelled observable, so commitPending stays true forever; the next save is then dropped by the take(1)+filter(!commitPending) guard in submitJsonPatchOperations, no SAVE_*_SUCCESS/ERROR is emitted, savePending stays true, and the form is stuck on "Saving..." until the user reloads (losing unsaved metadata). concatMap serializes the save effects so each inner observable completes (dispatching Commit/Rollback and clearing commitPending) before the next save runs, eliminating the race. Only the four save effects are changed (saveSubmission$, saveForLaterSubmission$, saveSection$, saveAndDeposit$), matching upstream. deposit/discard/updateSection intentionally keep switchMap. The improved catchError from DSpace#5145 is not backported (it depends on parseErrorResponse and action signatures not present in 8.3). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…nners Fixes a class of "spinner spins forever, user must reload and loses typed metadata" bugs in the submission form. A component sets a loading/searching flag, loads data via a RemoteData pipe, and resets the flag ONLY on the success path using getFirstSucceededRemoteData()/...Payload()/...ListPayload(). DSpace turns HTTP errors into a NON-throwing failed RemoteData (state=Error), and the store-backed RemoteData observable never completes, so on a failed OR slow/ never-responding request the success-only operator never emits, the component's catchError (which only catches thrown errors) never fires, and the spinner stays forever. There is also no HTTP request timeout anywhere in the app. Inherited from vanilla DSpace 8.3 and NOT fixed upstream (the same pattern is still present on main/10.x), so there is no backport available -- this is a new hardening fix, following the same getFirstCompletedRemoteData approach upstream used for the whole-form loading fix (DSpace#4060). For each affected load: switch getFirstSucceededRemoteData* to getFirstCompletedRemoteData() (emits on success AND failure, then completes), extract the payload failure-safely, add timeout({ each: 30000 }) for never-responding requests, and reset the spinner flag in finalize() so it always clears. On failure/timeout the control shows an empty result instead of hanging. Sites fixed (all inherited from 8.3): - sections/form/section-form.component.ts (Describe section) -- bounded 30s timeout so it never spins forever; kept getFirstSucceededRemoteData for the item follow-link to avoid dereferencing an undefined submission object. - ds-dynamic-form-ui/models/list/dynamic-list.component.ts (radio/checkbox vocab, e.g. dc.type) - ds-dynamic-form-ui/models/scrollable-dropdown/dynamic-scrollable-dropdown.component.ts (vocabulary dropdowns) - ds-dynamic-form-ui/models/onebox/dynamic-onebox.component.ts (authority typeahead) - ds-dynamic-form-ui/models/tag/dynamic-tag.component.ts (keyword/tag authority) - ds-dynamic-form-ui/models/lookup/dynamic-lookup.component.ts (lookup authority) - vocabulary-treeview/vocabulary-treeview.service.ts (hierarchical vocabulary tree) - collection-dropdown/collection-dropdown.component.ts (collection picker; parentCommunity was hanging reduce()) - submission/form/collection/submission-form-collection.component.ts (change-collection selector) - sections/cc-license/submission-section-cc-licenses.component.ts (CC-license section) - ds-dynamic-form-ui/models/relation-group/dynamic-relation-group.components.ts (relation chips on edit) Verified locally: `tsc --noEmit -p tsconfig.app.json` and `eslint` both pass with 0 errors on the changed files. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Deterministic reproduction on a local DSpace 8.3 stack (published dspace-8_x-test
backend images + this frontend). A Playwright script simulates a failing/slow
controlled-vocabulary endpoint by returning HTTP 500 for
/api/submission/vocabularies/{name}/entries, then opens the "Type" dropdown on a
new submission's Describe form.
- before: getFirstSucceededRemoteData ignores the failed RemoteData, so the "Loading..."
indicator stays visible forever (measured stuck for the full 14s observation window).
- after: getFirstCompletedRemoteData + finalize handles the failed RemoteData, the
spinner clears immediately and the dropdown shows "No results found".
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Deterministic reproduction on a local DSpace 8.3 stack. A Playwright script delays every submission PATCH by ~3s (models a slow save) and fires two overlapping autosaves (two edits of the autosave-on-change dc.title field). - before (base, switchMap): the second save cancels the in-flight first save mid-transaction; commitPending is orphaned true; the second save is dropped by the take(1)+filter(!commitPending) guard (only 1 PATCH ever leaves the client); savePending never resets -> footer stuck on "Saving..." for the full 18s window. - after (this PR, concatMap): the saves are serialized; the first completes and clears commitPending before the second runs; "Saving..." clears within ~4s. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…fter
Second reproduction covering the slow/never-responding path (not just HTTP 500).
The vocabulary /entries request is held open forever (models a hung socket).
- before (no timeout): "Loading..." stays stuck past 45s (never recovers).
- after (timeout({each:30000})): the spinner clears ~30s after the request stalls
and the dropdown shows "No results found".
Clips are sped up ~3x. Confirms the timeout() backstop half of the fix, which the
HTTP-500 reproduction did not exercise.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…tion is pending
The DataShare simple item view renders the DOI ("Persistent Identifier") field
via ds-item-page-uri-field, which filtered dc.identifier.uri for values starting
with https://doi.org. The whole field wrapper was gated behind *ngIf="hasDoiLink",
so when a record's DOI has not been registered yet (the DOI value is only written
to dc.identifier.uri by the backend AFTER the doi-organiser scheduled task
registers it), the field was hidden entirely and users saw no indication that a
DOI exists or is coming.
Restore the previous behaviour where the DOI field is always shown (with an empty
value while registration is pending):
- MetadataUriValuesComponent / ItemPageUriFieldComponent: add an opt-in `doiField`
input. In DOI mode the field wrapper is always rendered (hideIfNoTextContent =
false) and only https://doi.org values are shown as links. When false (default)
the upstream generic behaviour is kept, which also repairs non-DOI URI fields in
the base theme that were unintentionally hidden by the previous global filter.
- datashare untyped-item: opt the DOI field into `[doiField]="true"`.
Test-driven: added specs for the pending (no DOI yet), registered, and empty cases,
plus the default (non-DOI) URI behaviour.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ation
Follow-up to the loading-spinner hardening:
- Lower the request timeout backstop from 30s to 15s on all the hardened loads
(a 30s spinner is too long for an interactive dropdown).
- On a failed OR timed-out controlled-vocabulary / authority lookup, show a
dismissible error toast ("Something went wrong while loading the options.
Please try again.") instead of silently showing an empty "No results found",
so the user knows to retry. Added a shared notifyVocabularyLoadError() helper on
the DsDynamicVocabularyComponent base (via inject()) and wired it into the
scrollable-dropdown and lookup loads. A genuinely empty (succeeded) result is
unchanged -> still "No results found", no error toast.
- Harden dynamic-onebox vocabulary$ (findVocabularyById): getFirstSucceededRemoteDataPayload
-> getFirstCompletedRemoteData, so a failed vocabulary-metadata lookup resolves
to "not hierarchical" (no tree button) instead of leaving isHierarchicalVocabulary$
pending forever. Null-safe guards on result?.hierarchical and vocabulary?.preloadLevel.
tsc --noEmit and eslint pass with 0 errors. Verified in the local repro: on a 500,
the Type dropdown now clears immediately, shows "No results found", AND a red
"Something went wrong ... Please try again." toast appears.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ast behaviour The after clip now shows the updated recovery: on a failed vocabulary load the Type dropdown clears immediately, shows "No results found", and a dismissible "Something went wrong ... Please try again." error toast appears. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…(was 30s) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… wrapper Address review nits on the pending-DOI fix without changing behaviour: - Merge the doiField and default template branches into one ds-metadata-field-wrapper ([hideIfNoTextContent]="!doiField"), removing the duplicated link markup. - Add a `doiValues` getter that filters mdValues to https://doi.org values (restoring the `typeof value === 'string'` guard). In DOI mode the *ngFor and the `!last` separator are computed against this visible-DOI subset, so a trailing non-DOI value can no longer emit a stray separator after the last DOI. - Tighten the registered-case spec to assert the label header, and add a regression spec for multiple DOIs followed by a non-DOI value (only one separator between the DOIs). Verified locally: metadata-uri-values + item-page-uri-field specs 23/23 green, lint clean, build:prod (SSR production build) exits 0. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The DsDynamicVocabularyComponent base now injects NotificationsService (for the "something went wrong" error notification on a failed vocabulary load). Its subclasses' unit tests (scrollable-dropdown, onebox, tag, lookup) did not provide it, so component construction failed with "NullInjectorError: NotificationsService -> Store -> No provider for Store" (NotificationsService is providedIn root and depends on the ngrx Store, which the isolated TestBeds don't set up). Provide the existing NotificationsServiceStub in each of the four specs. Verified locally: those 4 spec files now run 54/54 SUCCESS (were 54 FAILED). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The before/after clips are now hosted on the separate 'pr-repro-media' branch and referenced from this PR's description, so they are no longer part of the code diff. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The before/after clips are now hosted on the separate 'pr-repro-media' branch and referenced from this PR's description, so they are no longer part of the code diff. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…d-coding it The DOI field decides which dc.identifier.uri values are DOIs by matching the DOI resolver base URL. That URL is a backend setting (identifier.doi.resolver), so read it from the REST config endpoint instead of hard-coding "https://doi.org" in the UI. - MetadataUriValuesComponent now fetches identifier.doi.resolver via ConfigurationDataService on init (only in doiField mode) and uses it in doiValues. Falls back to the DSpace default https://doi.org (DOIServiceImpl#RESOLVER_DEFAULT) when the property is not exposed/defined, so it stays correct without any backend change. - Added a spec proving a value matching a custom configured resolver is treated as a DOI, and provided ConfigurationDataService stubs to the dependent specs (item-page-uri-field, publication, untyped-item). Requires the backend to expose identifier.doi.resolver in rest.properties.exposed (separate backend PR); until then the frontend uses the default resolver. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ending
Instead of showing an empty DOI ("Persistent Identifier") field while the DOI is still
queued for registration by the scheduled task, show a "DOI registration in progress"
message. Once registration completes and the resolver value appears in dc.identifier.uri,
the DOI link is shown instead.
- metadata-uri-values: in doiField mode, render the item.page.doi.pending message when
there is no DOI value yet (doiValues is empty).
- add the item.page.doi.pending i18n key to the datashare theme and core en.json5.
- specs: assert the message is shown when pending / no identifier, and NOT shown once a
DOI is present.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
dspeed2
merged commit Jul 9, 2026
508977c
into
UoEMainLibrary:datashare-UoEMainLibrary-dspace-8_x
5 checks passed
dspeed2
pushed a commit
that referenced
this pull request
Jul 27, 2026
* UoE/datashare: check 'Inherit policies' by default on the move screen Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * UoE/datashare: make clear the Keep-embargo option is about the item's existing embargo Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * UoE/datashare: add "Keep embargo policies" option to the move-item screen UoE/datashare: add "Keep embargo policies" option to the move-item screen * feat: auth gate for admin control of bitstream access conditions * fix: remove redundant error handling * fix: patch the JSON patch success handler to clear isSaving before early return * feat: Hide the admin sidebar for regular authenticated users while preserving it for authorized admin roles * fix(admin-menu): skip authz calls for anonymous, fix lint - gate the 5 admin-panel authorization requests behind isAuthenticated: anonymous bootstraps (incl. SSR renders) make zero authorization calls again - extract ADMIN_PANEL_FEATURES const with rationale for the deliberately excluded features (CanSubmit/CanEditItem/CoarNotifyEnabled) - rename createAdminMenuIfLoggedIn$ -> createAdminMenuIfAuthorized$, fix stale doc comment - fix lint: import sort/newlines, EOF newline - specs: anonymous-user cases for resolver + sidebar; fix AuthService stub provider (useValue class ref -> useClass) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(submission): fail-closed admin check, repair spec teardown crash - default canEditAccessConditions to false: authorization gates must fail closed until the site-admin check resolves - spec: authorization mock now returns an Observable (was a bare spy function -> '.pipe of undefined' crash when the TestComponent template ran ngOnInit) - spec: stub the component's ChangeDetectorRef in the ngOnInit tests; rendering the full dynamic form against the mocked TestBed caused lazy NullInjectorError (HttpClient) in afterAll, which disconnected Karma and killed the whole CI test run - spec: restore compileComponents() promise chaining - spec: add explicit admin-path test (form model contains accessConditions array for authorized users) and set canEditAccessConditions=true in the tests that build the form directly Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(submission): reset isSaving when the save PATCH fails Address review feedback: - add error handler to the jsonPatchByResourceID subscription so a failed PATCH no longer leaves the modal stuck in the saving state (+ regression spec) - soften the new authorization specs to assert behavior (presence/ absence of the accessConditions control) instead of exact form model shape Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Fix admin sidebar visibility regression and reduce scope - remove one-shot auth sampling and keep reactive auth/admin check in admin sidebar - gate sidebar on site-admin authorization only (FeatureID.AdministratorOf) - revert resolver/test/stub changes to match upstream behavior - remove admin panel visibility utility and non-standard test spy alias Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix MenuService test stub method signatures Align showMenu/hideMenu stub signatures with optional MenuID parameter so admin sidebar specs compile. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * UoE/datashare: Fix home-page reload flicker (hydration-safe SSR anti-flicker overlay) Reported: the page visibly flickers on reload. Root cause: provideClientHydration() is enabled, but DSpace's ThemedComponent (header/navbar, footer, ...) builds its real content imperatively via ViewContainerRef.createComponent() inside a client-side, async ngAfterViewInit. Angular hydration cannot reuse an imperatively-created component, so that subtree is destroyed and re-created client-side, visibly, after the rest of the page has already painted (e.g. the navbar pops in ~500ms after the banner/search box, pushing all content down). Same class of bug as upstream DSpace#3867. Fix, ported from customer/mendelu's Angular-18 hydration-safe variant of the same fix already shipped for other customer instances on this org's Angular-15/DSpace-7 branches: - src/index.html: inline bootstrap script clones (never moves - a move would break hydration on this Angular-17 app) the SSR-painted <ds-app> into a detached, opaque overlay before Angular's bundle runs. The overlay sits on top while hydration + the imperative themed re-render happen invisibly underneath on the untouched original, and self-disables for Cypress/WebDriver-flagged automation. - src/app/app.component.ts: removeSsrOverlayWhenContentVisible() drops the overlay once the auth/theme loader gate has opened AND the live <ds-app> DOM has settled (no element added/removed for 600ms, with real content present) - not on ApplicationRef.isStable, which stays busy long after the page is visually done. 15s hard fallback lives in index.html. - src/typings.d.ts: Window.__dspaceRemoveSsrOverlay global the two files above share. - src/app/app.component.spec.ts: regression test for the gate+settle removal logic. Reproduced and verified locally in Docker (Dockerfile.dist SSR build) against a local DSpace 8.x REST backend - see flicker-evidence/README.md for methodology and before/after screenshots. * test(admin-sidebar): run ngOnInit once per authz test, guard take(1) regression Restructure the admin-sidebar authorization specs so each test configures its stubs/spies and then triggers a single ngOnInit via a fresh component, removing the manual comp.ngOnInit() that ran after the shared detectChanges(). Addresses the duplicate-subscription review feedback (Copilot) on the three tests. Add a regression test that drives isAuthenticated() as a BehaviorSubject (false -> true) and asserts the admin menu is hidden then shown, so a re-introduced take(1)/first() on the auth gate would fail the suite. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat: include file name in submission upload notifications Multi-file drag-and-drop uploads showed generic 'Upload successful'/'Upload failed' toasts, so users could not tell which file each notification referred to. Add a non-breaking onCompleteItemWithFile output on UploaderComponent that carries the client-side file name alongside the parsed response, and use it (plus the existing error payload) in SubmissionUploadFilesComponent to render per-file notifications, falling back to the generic keys when no name is available. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix: make UploaderCompleteEvent fileName optional to match emitter The emitter can pass undefined when the FileItem carries no file name; the interface claimed it was always present. Also cover the missing-file-name emit in the uploader spec. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * refactor: address Copilot review on upload notification typing Omit fileName from UploaderCompleteEvent when unknown instead of emitting an explicit undefined, type the onUploadError output as EventEmitter<UploaderError>, and drop the explicit undefined fileName from the fallback spec. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix: prevent duplicate values in repeatable submission dropdowns Selecting a value already present in another row of the same metadata field (e.g. the Funder dropdown) is now blocked: the change is reverted to the previous value and a warning notification is shown. Guard is scoped to scrollable dropdown models only. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(statistics): clear statistics table pagination params on destroy Statistics pagination is URL-driven with queryParamsHandling: 'merge', so a page/rpp selection made on one scope's statistics page persisted in the URL when navigating to another scope. Clear the table's own params on destroy, like other paginated components do. Verified on the seeded local instance that ds-pagination already renders and pages correctly at community, collection and item level via the shared StatisticsTableComponent (DSpace#726); no further frontend change is needed for lower-level pagination. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(statistics): explain deferred pagination-param cleanup accurately The stale stats-* params only survive 'merge' navigations (e.g. the navbar search form); plain navigations drop them. Clarify that clearPagination stages nulls applied on the service's next updateRoute, and why no navigation is issued from ngOnDestroy (racing the in-flight navigation). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test(statistics): destroy uninitialised table via the framework path Let Angular invoke ngOnDestroy through fixture.destroy() on a component whose ngOnInit never ran (createComponent without detectChanges), instead of calling the hook directly. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(submission): disable already-used options in repeatable dropdowns Gray out (dsBtnDisabled + gray-200 background) and block options already selected in another row of the same repeatable field, so duplicates cannot be picked at all - mouse or keyboard. Also fix the duplicate-guard revert leaving the rejected value on screen: setCurrentValue before dispatchUpdate, no distinctUntilChanged on the control subscription, explicit model.value revert in the guard. Part of dataquest-dev/dspace-customers#624 (items 001, 005). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * style(submission): force disabled-option styling and normal hover cursor Add !important to the disabled dropdown-option background/color so they win over Bootstrap's .dropdown-item.disabled, and re-enable pointer-events with a default cursor so hovering a used option shows a normal cursor (selection stays blocked by the dsBtnDisabled/onSelect guards). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(submission): refresh used sibling values on keyboard-open selectOnKeyDown opened the dropdown with sdRef.open() directly on Enter, bypassing openDropdown() so usedSiblingValues (and the option reload/page reset) were stale -> duplicates could be selected via the keyboard-open path. Route through openDropdown() instead. Addresses Copilot review on PR DSpace#26. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * refactor(submission): drop superseded duplicate-value guard, keep disabled-option prevention * removed emptyline * UoE flicker: block pointer-events on the frozen clone (Copilot); keep clone ids Incorporates the applicable Copilot review point from the sibling PR DSpace#16 onto this hydration-safe clone overlay, and records the reasoned outcome of the second one. - pointer-events: the full-viewport SSR freeze-frame now intercepts pointer input (pointer-events: auto) instead of letting clicks pass through to the still-hydrating live app the user cannot see; flipped to none the instant fade-out starts so the revealed app is interactive for the whole 150ms crossfade. - id-stripping (Copilot's other suggestion) is intentionally NOT applied: stripping ids from the clone unstyles every id-gated rule on it (global #main-content flex; header / #main-navbar under emulated encapsulation), which measured 40.65% clone-vs-final mismatch locally and turned the fade-out into the very jump the overlay removes. The clone keeps all ids and stays pixel-identical (0.4% removal diff); duplicate ids are harmless because the clone follows <ds-app> in document order. Rationale in the code. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * UoE flicker: add local Docker reproduction + fix-verification evidence Independent, fully local repro (stock DSpace 8 backend on a clean DB in Docker + this branch's production SSR build) with high-fps CDP screencast capture. Quantifies the navbar-band deviation from the settled page: before (overlay off): navbar entirely missing mid-reload, up to 73.9% deviation, ~2.17s after (overlay on): frozen clone holds the full navbar, <=0.4% deviation, 0ms flicker Includes before/after/diff screenshots, methodology, and the id-stripping measurement. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Drop committed flicker screenshots from the PR — keep it source-only Binary before/after screenshots don't belong in the source tree. The reproduction methodology and the quantified before/after numbers live in the PR discussion instead. Net PR diff is now source-only (index.html, app.component.ts, app.component.spec.ts, typings.d.ts). * UoE flicker: also mask the post-login white flash (unify overlay via fullscreen-loader watch) PR DSpace#23 masked the home-page RELOAD flicker but not the POST-LOGIN flash: login hard- redirects through /reload/<ts> -> /home and, while auth is re-established, root.component.html hides the whole .outer-wrapper (incl. #main-content) via `d-none` and paints .ds-full-screen-loader (white screen + spinner) -- on the current page right after submit AND across the reload. The old overlay was gone by then, so the user saw a ~1s white flash + content shift (admin sidebar). Verified locally in Docker (before: ~1.5s white; after: eliminated). This rewrites the inline bootstrap into one self-contained mask lifecycle that handles both: - installMask(): freeze a detached CLONE of the last real SSR/routed view (still never moves the DOM Angular hydrates; still keeps all ids so the frozen frame is pixel-identical). - A session-long MutationObserver re-masks with the last remembered real frame whenever the fullscreen auth/theme loader appears unmasked (covers login/logout, not just first load). - revealWhenPainted(): lift (150ms fade) only once the routed page is genuinely painted -- no .ds-full-screen-loader, #main-content AND the themed navbar have real layout height -- and the DOM has been quiet for 800ms. The navbar/settle gate is what stops an intermittent reload FOUC (revealing over a half-built/unstyled page). 15s hard cap per mask. - window.__dspaceRemoveSsrOverlay now just nudges the reveal, so AppComponent's existing DOM-settle hook (unchanged) still works; self-disables under Cypress/webdriver as before. Verification: 12/12 logins and 8/8 reloads with zero blank/FOUC frames locally (only the browser's own ~1-frame inter-document reload gap remains, which no in-page script can mask); independently reviewed by 3 UX passes over before/after filmstrips -> all APPROVED. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * UoE flicker: render the desktop header on the server (fix reload content-shift) The home-page reload showed a content-shift independent of login state: the header's "Log In" control changed from a plain link to the dropdown button and the search box resized on every load. Root cause: <ds-auth-nav-menu> (and other responsive components) branch on HostWindowService.isMobile(), whose width observable stayed empty during SSR because hostWindow.reducer's initial width was null (HostWindowService filters out null widths). So the server rendered the mobile branch and the client, once it measured the real desktop width, swapped to the desktop branch — shifting the header and resizing search. Fix: default the hostWindow width/height to a desktop viewport so SSR renders the desktop layout that matches the common (desktop) hydration; the browser overrides it with the real size immediately (StoreEffects.resize / AppComponent). Also guard AppComponent's window-size dispatch to the browser so the server no longer overwrites that default with an undefined size. With this the SSR markup matches the settled client page, so the anti-flicker overlay's frozen clone is pixel-identical to the revealed page (verified: reload 0/8 blank + 0/8 shift, clone==final, on the datashare theme). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * UoE flicker: update hostWindowReducer spec for the desktop default width The reducer's initial state now defaults to a desktop viewport (1200x800) instead of null so SSR renders the desktop layout (see prior commit). Update the spec that asserted the null initial state accordingly. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * UoE flicker: mask logged-in reloads with the matching auth state (kill the sidebar shift) A logged-in reload/refresh still shifted the whole body ~55px: the admin sidebar is only rendered client-side after hydration, so it re-appeared over the masked reload and the content reflowed at reveal. The overlay was masking with the pre-hydration SSR clone, which is anonymous (no sidebar), so revealing it exposed the shift. Fix the mask source selection so it matches the auth state the page settles to: - prefer the fresh SSR clone when it is already logged-in (the post-login /reload renders authenticated, sidebar included); - else the last SETTLED remembered frame when THAT is logged-in (a plain logged-in F5 reload, whose SSR renders anonymous but whose remembered frame has the sidebar); - else the remembered settled frame (or SSR clone on first load). The logged-in header user-menu (.fa-user-circle / .dropdownLogout) is the tell. Also remember the frame at reveal (once the live page has genuinely settled) rather than from the unsettled pre-hydration SSR, so the remembered frame is always the real final render. Verified on the datashare theme (cpu1): logged-in reload 0/3 shift (was ~55px), logged-out reload still 0/3, login no white flash and the sidebar no longer jumps. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * UoE flicker: drive the admin-sidebar gutter from CSS, not an animation The page's offset beside the admin sidebar came entirely from a padding-left applied by the @slideSidebarPadding animation: the sidebar is position: fixed, so it contributes no layout width of its own. That animation reads its width from the CSS-variable store, which is populated by scraping document.styleSheets -- something platform-server does not have. The server therefore rendered padding-left: 0 and the browser animated 0 -> 55px over 300ms, sliding the whole page sideways on every authenticated reload. Resolve the gutter from the --ds-admin-sidebar-* custom properties via CSS classes instead, so it renders identically on the server and in the browser with no hardcoded width, and keep the 300ms slide for genuine pin/unpin behind a separate ds-admin-sidebar-animate class. This mirrors the fix already carried on dataquest-dev/dspace-angular branch customer/vsb-tuo, with one adaptation: that branch arms the transition one requestAnimationFrame after first paint, which is enough on DSpace 7. DSpace 8 discards the server-rendered DOM and re-renders it ~350ms later, so the gutter briefly falls back to 'hidden' well after that first paint; arming on the first post-bootstrap 'visible' instead keeps that recovery instant. Measured on a live instance with the anti-flicker overlay both on and off -- measuring only with it on validates the overlay, not the fix: authenticated F5 17 animated frames (0 -> 55px over 280ms) -> 0 anonymous F5 0.0px -> 0.0px pin/unpin toggle animates, 17 frames -> unchanged Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * UoE flicker: reserve the scrollbar gutter so the page stops sliding on reload While Angular boots it replaces the server-rendered DOM, and for ~90ms the page is too short to scroll. The vertical scrollbar disappears and comes back, so the viewport widens from 1425 to 1440 and back, and every centred container slides half the scrollbar width to the right and then back to the left. The anti-flicker overlay cannot hide this one: the overlay lives inside the page, so it shifts along with everything else. That is why the reload still visibly jumped after the gutter/padding work -- it was never the admin sidebar. Reserving the scrollbar's width permanently makes its appearance and disappearance a no-op for layout. Measured on the real /home page, authenticated, in a headed browser (headless Chromium uses overlay scrollbars that occupy no width, so it cannot reproduce this at all -- and an earlier attempt measured the wrong page entirely, because login hard-redirects to the end-user agreement until that is accepted): full-frame horizontal shift, overlay on: +8px at 758-811ms -> 0px max frame difference vs the settled page: 8.1% -> 0.1-0.3% over three runs. The remaining sidebar-gutter flap during the component's re-creation stays covered by the overlay, which is what the overlay is for. Also nest a second animation frame before arming the gutter transition, so the arming can never share a style recalculation with the class change it must not animate. Defensive: the gutter applied instantly with a single frame in every run measured here. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * UoE flicker: stop the overlay eating clicks, and drop the dead half of it Three defects the earlier commits introduced or left behind: The overlay set `pointer-events: auto` deliberately, reasoning that a click on a frozen clone must not reach the live app behind it. In practice that made the whole page unresponsive for as long as the mask was up: you click a nav link and nothing happens, which reads as a broken site. It now never absorbs input -- the click goes through to the live app -- and the mask lifts on the first interaction, so you see the result of the click instead of a picture of the previous page. `scrollbar-gutter: stable` collided with ng-bootstrap, which pads the body by the scrollbar width when a modal opens to compensate for the scrollbar it hides. With the gutter reserved permanently that width is never freed, so the compensation was pure surplus and shifted the page 7.5px left on every modal -- in a change whose purpose is removing layout shifts. Cancelled for `body.modal-open`. The remembered frame was stored under one unkeyed slot, so a hard navigation or a logout could paint the previous page -- including logged-in chrome -- over the new one. It is now keyed by URL and only reused on a match. Also removes ~230 lines that never did anything: AppComponent's overlay driver, its spec and its Window type augmentation all existed to call `window.__dspaceRemoveSsrOverlay`, which is `function () { revealWhenPainted(); }` -- the same function index.html already re-schedules every frame until the page settles. The two sets of tuning constants (600ms/10s vs 800ms/15s) could never take effect. Deleted along with the global itself. Verified on the running instance, headed, on the real /home page: incognito, click 300ms after load click swallowed -> navigates modal open content -7.5px -> 0px authenticated F5 0px -> 0px (unchanged) navigation between pages 0px -> 0px cold login (home never visited) no white flash -> no white flash Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(submission): say when a file is being ingested instead of just "Saving" An ingest from a server path happens inside the save request, so for a large dataset the submission form sat on a bare "Saving..." for minutes with nothing to say a transfer had started or whether the tab could be closed. While a save carries a pending local.bitstream.redirectToURL value the progress bar now reads "Ingesting file from server..." with a running mm:ss counter, alongside a note that the file is being read directly from the server and the tab should stay open. Ordinary saves are unchanged. The elapsed time is measured from wall-clock timestamps rather than by counting timer ticks, because browsers throttle timers in a backgrounded tab - exactly where a long ingest is left running. The pending state is latched when the save is dispatched, so the label describes the request actually in flight rather than a form that has since changed, and a pending removal of the field wins over a value still sitting in the section data, so clearing the field is not reported as an ingest. "Saving..." and "Depositing..." were hard-coded English in the template and are now translated like everything else. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * UoE flicker: move the overlay script out of index.html into a source file The anti-flicker overlay has to run as a synchronous inline script before the deferred module bundles, so it cannot live inside the Angular app. But it does not have to be pasted into index.html as a 200-line blob. Keep the source in src/anti-flicker-overlay.js -- a normal, formatted, editable file -- and inline it into index.html at build time via the build's indexTransform hook (webpack/index-html-transform.ts), which the custom-webpack builder already supports. The built index.html is unchanged: the script is still inlined, still ahead of the module bundles, same runtime behaviour. Only the source is cleaner -- index.html goes from 226 lines to 38, and the overlay logic reads like code instead of markup. The transform throws if the marker is missing, so the overlay can never silently drop out of a build. Verified after the change, on the running instance: built dist/index.html overlay inlined ahead of the module scripts, marker gone SSR-served /home overlay present inline incognito, click 300ms in navigates (click passes through) authenticated F5, headed 0px Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(submission): remove obsolete "same file name" upload warning and block Two files with the same name are handled correctly in the DSpace 8.3 backend (the DataShare dataset zip disambiguates duplicate entry names via uniqueEntryName(), and per-file download is UUID-addressed), so the submission UI no longer needs to warn about - or block deposit on - duplicate file names. Removes the DataShare duplicate-file-name detection end to end while keeping the separate total-upload-size feature untouched: - section-upload.component: drop the duplicate warning message + duplicate detector wiring; keep the total-size calculation and its warning - submission-form-footer.component: restore the vanilla deposit() (no longer gated by the duplicate check / "cannot submit" notification) - datashare-submission.service: remove duplicate-detection helpers, the shared hasUploadFilesErrors signal and sendCannotSubmitNotification; keep the file-size helpers - i18n (en + datashare theme): remove the now-unused datashare.submission.sections.upload.duplicates and .submit.errors keys - specs: drop the removed mocks/providers Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(submission): remove obsolete "same file name" upload warning and block Two files with the same name are handled correctly in the DSpace 8.3 backend (the DataShare dataset zip disambiguates duplicate entry names via uniqueEntryName(), and per-file download is UUID-addressed), so the submission UI no longer needs to warn about - or block deposit on - duplicate file names. Removes the DataShare duplicate-file-name detection end to end while keeping the separate total-upload-size feature untouched: - section-upload.component: drop the duplicate warning message + duplicate detector wiring; keep the total-size calculation and its warning - submission-form-footer.component: restore the vanilla deposit() (no longer gated by the duplicate check / "cannot submit" notification) - datashare-submission.service: remove duplicate-detection helpers, the shared hasUploadFilesErrors signal and sendCannotSubmitNotification; keep the file-size helpers - i18n (en + datashare theme): remove the now-unused datashare.submission.sections.upload.duplicates and .submit.errors keys - specs: drop the removed mocks/providers Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * UoE flicker: minify the overlay when inlining it into index.html The overlay must be inlined (it runs before the deferred bundles), so it always appears in the served HTML — "view source" showed 200 readable lines. Minify it with terser at inline time: the served page now carries a single ~2.7kB line with a one-line banner pointing back to the source. src/anti-flicker-overlay.js stays the readable, formatted source; only the browser copy is compacted. Runtime behaviour is unchanged. Verified: served /home inline script is 2 lines (banner + minified body) instead of ~200; the overlay logic is intact (mask installs, click passes through, reveal works). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(item-page): stop 500 on "show more" for items with many bitstreams The datashare theme already renders CC-LICENSE/LICENSE in its own Licences section pinned to page 1. The base FileSectionComponent also fetched CC-LICENSE in lockstep with ORIGINAL on every "show more" click, using the same page number while isLastPage was derived from ORIGINAL only. Once ORIGINAL had more pages than CC-LICENSE, later clicks requested a CC-LICENSE page past its end, which the REST API answered with HTTP 500 - and it also rendered the licence file twice. Fetch ORIGINAL only here. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * refactor(datashare-submission): address Copilot review on total-size helpers - MAX_FILE_SIZE_GB/BYTES -> private readonly (used only inside the service; they are constants) - formatBytes: add TB/PB units and clamp the unit index so values >= 1 TB no longer render "undefined" Behaviour for in-range values (<= 20 GB upload cap) is unchanged; verified across ranges. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(item-page): revert getNextPage error handling to upstream Addresses Copilot review: clearing isLoading on the error branch enabled a retry that requested the next page rather than re-requesting the failed one, because currentPage is incremented up front. That error-handling tweak was not needed for the 500 fix, so getNextPage now matches upstream exactly (isLoading cleared only on success) - the fix is solely dropping the CC-LICENSE fetch. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * UoE flicker: slide the admin sidebar in on login (content stays put) A genuine login now animates the admin sidebar into view. The auth effect flags the login (one-shot, never on a plain reload); the anti-flicker overlay, as it lifts on the /home the login lands on, adds a body class that runs a CSS keyframe sliding the sidebar in from the left. Crucially this is NOT the vanilla gutter animation, which slides the page content 55px — that is the same shift removed from the reload path, and bringing it back would reintroduce exactly that. Here only the fixed sidebar's transform animates; the content and its reserved gutter never move. So the entrance is visible on login while reload stays perfectly still. The flag is consumed only on a reveal that sticks (no re-mask within 250ms): login redirects through /reload -> /home with several mask/reveal cycles, and an earlier transient reveal must not eat the one-shot flag before the final, visible one. Respects prefers-reduced-motion. Measured, headed, on the real pages: login sidebar slides -55 -> 0, visible after the mask lifts; content 0px (3/3 runs) authenticated F5 content 0px, no entrance animation (flag absent on reload) anonymous F5 0px modal open content 0px click through mask navigates Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(item-page): document base-vs-theme licence rendering; pin spec Expert review follow-ups: - Class doc on FileSectionComponent stating it lists ORIGINAL only and that licence bundles (CC-LICENSE/LICENSE) are rendered by the theme, so the base never fetches them (paging them in lockstep past their end caused the 500). - Spec now asserts findAllByItemAndBundleName is called with ORIGINAL and never with CC-LICENSE/LICENSE, so a regression re-introducing the second fetch fails. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Revert "UoE flicker: slide the admin sidebar in on login (content stays put)" This reverts commit 6ad726e. * fix(submission): show an actionable error when a server-path ingest fails Entering a path that does not exist (or a URL, or a path outside the allowed directories) failed the save with the generic "There was an issue when saving the item, please try again later." - which gives the administrator nothing to act on, since the real cause is the path they typed. The backend returns a 400, but DSpace genericises the message and the save effect discards the error, so the specific reason cannot reach the UI. Instead, the saveError effect now checks whether the failed save carried a pending local.bitstream.redirectToURL value - the rolled-back operations still hold it at that point - and, if so, shows a message that says what to check: an absolute path, to a file that exists on the server, inside an allowed directory, and not a URL. Every other save error keeps the original notice. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(submission): shorten the upload-from-path error to a single plain sentence The earlier message spelled out every possible cause; a plain 'the file could not be found at the path you entered' is enough. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(item-page): isolate ORIGINAL-only assertion to the click Copilot review: detectChanges() runs ngOnInit -> getNextPage, so the spy is already called before the click. Reset the spy first so the assertions verify the click behaviour, not ngOnInit. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Merge remote-tracking branch 'origin/datashare-UoEMainLibrary-dspace-8_x' into uoe/bitstream-upload-from-path * test(item-page): assert requested bundle names directly Copilot review: jasmine.anything() does not match undefined (comp.item is unset), weakening the negative assertions. Capture the bundle-name argument of every call and assert it equals exactly ['ORIGINAL'] - independent of the item argument, and failing if any licence bundle is fetched. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * refactor(datashare): type the tri-state path outcome as boolean | undefined Address Copilot review: pendingOutcome and pathOutcome genuinely return a third state (undefined = no pending operation touches the field) alongside true/false, so declaring them boolean masked that. Typing them honestly and narrowing with a type guard makes the undefined branch explicit. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(item-page): fix misleading bitstream pageSize comment The old comment claimed 'override to 5 (default) as per config.prod.yml', but the code sets pageSize = 25 and no config.prod.yml sets item.bitstream.pageSize (its default is 5). Reword to state the value is hard-coded to 25 on purpose to load more files per 'Show more' click. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * UoE flicker: declare terser as an explicit devDependency webpack/index-html-transform.ts imports terser directly (to minify the inlined anti-flicker overlay), but terser was only present transitively via the Angular build tooling. Declare it explicitly so the build does not rely on hoisting. Pinned to 5.29.1 to match the existing yarn.lock resolution; `yarn install --frozen-lockfile` passes with no lockfile change. Addresses a Copilot review note. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * chore(submission): shorten the datashare comments Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(comcol): widen collection/community logo max-width so wide banners aren't squeezed The upstream `--ds-comcol-logo-max-width: 500px` cap limits the logo's *width*. Wide banner-style logos are therefore squeezed into a thin strip: e.g. the RESPIRE collection logo (6000x1100 PNG, 5.45:1) renders at only ~500x92, which users report as a "tiny logo". Raise the width cap to 100% so wide logos fill the header/content column. The `--ds-comcol-logo-max-height: 500px` cap is kept, so tall/square logos are still bounded and only genuinely wide banners get larger. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(comcol): enlarge result thumbnail width to 175px Result-list/detail thumbnail width is capped by --ds-thumbnail-max-width (125px); customer reports thumbnails look small. Raise to 175px, which matches the backend-generated thumbnail resolution (thumbnail.maxwidth = 175), so it stays crisp with no backend change / thumbnail regeneration needed. Height (--ds-card-thumbnail-height) is intentionally left at 240px. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * UoE/feat(mydspace): rename submission menu and status labels (LUC070-260) Renames the /mydspace "Show" options, the page headings beside them and the Status filter values, and fixes the applied filter showing a raw Solr token instead of a readable label. - "Show" dropdown: "Submissions" -> "My submissions", "Workflow tasks" -> "Submissions – in review" - Status facet: "Workspace" -> "In draft", "Workflow" -> "In review", "Validation" -> "For validation". "Archived" and "Waiting for reviewer" keep their names, as the ticket asks - Result-card badges and the Administer Workflow screen follow the same wording, so a row cannot say "In draft" in the filter and "Workspace" on its badge - Avatar menu, breadcrumb, page title and "Back to Submissions" move to "My submissions" together, so the path to the page reads consistently Ticking a Status value used to relabel the checkbox and the applied filter chip with the raw token ("item" for Archived, "waitingforcontroller" for Waiting for reviewer). The unticked facet list keys off FacetValue.value, which cerialize aliases onto the backend label; the ticked option and the chip key off AppliedFilter, which carries the Solr token untouched. Adding the lower-cased token aliases makes all three render the same label, with no component change. The screen-reader announcement and the chip aria-label passed the same raw token through; both now resolve it through the key the visible text already uses. Also supersedes the December 2025 naming scheme left behind in the DataShare theme file. It mapped workspace -> "In review" and workflow -> "Draft" (the wrong way round), was reverted from src/assets/i18n/en.json5 in 457b86a, and declared two keys twice. It is inert today because merge-i18n is wired into no build step, but one manual run would have reinstated it over this change. "Supervised items" is deliberately left untouched, pending the customer decision on what it should be called or whether to hide it. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(submission): harden duplicate detection in repeatable scrollable dropdowns - key duplicate detection on authority (fallback value) so authority-controlled vocabularies (e.g. Funder) no longer accept the same entry twice - re-check live sibling values at commit time (selectEntry) to close the stale-set gap when a value is chosen in another row after opening - block mousedown on disabled options (dsBtnDisabled); options commit on mousedown - guard keyboard Enter against selecting while options are still loading - expose aria-selected on options - add reproduction/regression spec and a self-contained before/after demo * style: satisfy lint (indentation + import sort) * a11y: bind aria-selected to the selected value; clarify 1e spec name (review) * fix(i18n): restore keys dropped by the keep-embargo cherry-pick 4b21906 resolved the en.json5 conflict by taking the source branch's whole file. That branch (uoe/move-keep-embargo-policies, cut 2026-06-30) predates three keys the customer already had from sync PR #6, so the cherry-pick silently deleted them: form.vocabulary.load-error item.page.filesection.checksum item.page.doi.pending All three are referenced by code shipped in this PR, and MissingTranslationHelper renders the raw key, so the UI would show "item.page.filesection.checksum" twice per bitstream on the full item record and "item.page.doi.pending" on items awaiting DOI registration. The datashare theme copy does not help: angular.json never copies src/themes/*/assets/i18n into the build and merge-i18n is a manual script, so src/assets/i18n/en.json5 is authoritative at runtime. Unit tests cannot catch this - TranslateLoaderMock returns {}, so the specs assert on the raw key and pass either way. en.json5 is taken whole from the source branch; the two-dot diff on that path is additions-only, so this restores the three keys and changes nothing else. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * chore: keep local-only test Dockerfile and demo harness out of the PR Cherry-pick of dataquest-dev/uoe-dspace-datashare-angular 85124a0, authored 17 minutes after the last commit picked into this sync PR and so left outside the pick window. It is the only non-merge commit from the sync window that did not make it across. 2bc0bd6 faithfully carried Dockerfile.karma and demo/** over together with the dropdown fix; upstream then removed them again as local-only recording evidence. Without this commit the sync ships 13 files / 1,574,221 bytes - 98.7% of it incompressible .webm/.gif/.png - and git history is append-only, so a later deletion would not reclaim it. Dockerfile.karma also sits at the repo root with no CMD/ENTRYPOINT and an unpinned node:20-bookworm base, and demo/package.json is a second, unmanaged manifest that dependency scanners would pick up. Note: this keeps the blobs out of the customer's history only under a squash merge, as every previous sync PR was merged. Under a plain merge commit or a rebase merge, 2bc0bd6 still introduces them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(submission): stop repeatable dropdowns duplicating and clearing values Follow-up to DSpace#26 and DSpace#32. Both reported problems still reproduced on datashare-UoEMainLibrary-dspace-8_x, because they are not caused by the dropdown widget alone. Deleting a row silently rebound the surviving rows to the wrong controls ------------------------------------------------------------------------ getControlOfGroup() stamped a `startingIndex` on a group model the first time the row rendered and resolved that row's FormGroup as control.get([startingIndex]) from then on. Nothing re-synced it, while DynamicFormArrayModel re-indexes its groups on every insert/remove/move and the template binds formGroupName to that live index. After deleting a non-last row the two disagreed: surviving rows resolved to another row's control or to null, and after delete+add two rows could alias onto the same control. That is what let a value the user never touched be overwritten, and what made the duplicate check read stale sibling values. The live index is now the single authority. Drag and keyboard reordering moved the group models only, which is what the frozen index had been compensating for, so both now move the control alongside the model (moveGroupAndControl). DsDynamicFormControlContainerComponent.ngOnChanges also dereferenced a null `group` in the tick after a row was removed, throwing inside change detection and aborting the pass for the rest of the field. A click aimed at dismissing the menu erased the value ----------------------------------------------------- The menu is a full-width overlay drawn over the field's own bottom edge and the rows beneath it, and its first entry was the destructive "Clear selection" - so the spot a user naturally clicks to dismiss the dropdown wiped their selection, row after row. Clearing now sits at the end of the menu, visually separated, and the menu keeps a small gap below the input. The clear entry also carried both (click) and (mousedown) and therefore fired twice per interaction; it now has a single handler, like the options. Options deliberately keep committing on (mousedown): blurring the input flips showErrorMessages on a required field and forceShowErrorDetection() destroys and re-creates the control, so the element is gone before a click event could reach it. This is now documented in the template. Duplicate detection ------------------- usedSiblingValues was a snapshot refreshed only in openDropdown()/selectEntry(), so it went stale whenever a row was added, removed or edited - and the toggle caret opens the menu through NgbDropdown without calling openDropdown() at all, leaving the set empty and nothing greyed out. It is now derived from the live models, and every path that opens the menu refreshes the options via (openChange). DSpace#32's canonicalKey() keyed on `authority ?? value`, but the same entry arrives as a VocabularyEntry when picked in-session and as a FormFieldMetadataValueObject when rebuilt from stored metadata, and only one side may carry an authority. The keys were then incomparable and the duplicate went through. Identity is now the authority *and* the normalised value, and a match on either is a duplicate. Testing ------- 14 specs written first against the unfixed code and confirmed failing, covering the row/control binding across remove/insert/reorder and every duplicate and clearing path. Full suite: 5478 passing. Verified end to end on a dockerised DSpace 8 backend with the datashare theme: the three Type rows survive the dismissing click, the still-used values are greyed out after delete+add (input and caret alike), and what the server stores matches the form with no duplicate and nothing destroyed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(submission): keep keyboard reorder-cancel and clear-entry consistent Both from the Copilot review of this PR; both are regressions this PR introduced. cancelKeyboardDragAndDrop() still moved the group models only, so Escape after a keyboard reorder restored the model order while leaving the FormArray as it was - the exact desync this PR set out to remove. It now goes through moveGroupAndControl() like the drag and arrow-key paths. Dropping the clear entry's (click) handler to stop it firing twice per mouse interaction also removed its keyboard activation, since a native button turns Enter into a click. It now carries (keydown.enter) alongside (mousedown), which mirrors how the options are bound. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> Co-authored-by: Juraj Roka <95219754+jr-rk@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: MatusBeke <matus.beke7@gmail.com> Co-authored-by: milanmajchrak <minptai7@gmail.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
3 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.
This PR contains the following fixes:
Fix: Show
DOI registration in progresson the Item View when the DOI has not been registered yet.Fix: Propagated the fix from Vanilla. When select options take more than 15 seconds to load, an error message is shown asking the user to try again.