-
Notifications
You must be signed in to change notification settings - Fork 437
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fixed ItemSearchResultListElementComponent not fully themable #2651
Fixed ItemSearchResultListElementComponent not fully themable #2651
Conversation
…ResultListElementComponent from the themes folder when present
Hi @alexandrevryghem, |
…ute-main # Conflicts: # src/app/shared/mydspace-actions/claimed-task/switcher/claimed-task-actions-loader.component.ts # src/app/shared/object-collection/shared/listable-object/listable-object-component-loader.component.spec.ts
Hi @alexandrevryghem, |
…ute-main # Conflicts: # src/app/shared/object-collection/shared/listable-object/listable-object-component-loader.component.spec.ts # src/app/shared/object-list/item-list-element/item-types/item/item-list-element.component.spec.ts # src/app/shared/object-list/item-list-element/item-types/item/item-list-element.component.ts
Hi @alexandrevryghem, |
dd5f8f1
to
c476c4b
Compare
Hi @alexandrevryghem, |
…ute-main # Conflicts: # src/app/shared/object-collection/shared/listable-object/listable-object-component-loader.component.spec.ts # src/app/shared/object-list/item-list-element/item-types/item/item-list-element.component.spec.ts
Hi @alexandrevryghem, |
…ute-main # Conflicts: # src/app/shared/object-collection/shared/listable-object/listable-object-component-loader.component.spec.ts
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@alexandrevryghem : For some reason, with this PR installed, SSR hangs indefinitely. It's possible this has conflicts with recent changes like #2865
To reproduce:
- Build this PR in production mode by pulling this code on top of latest
main
and running:yarn install; yarn build:prod
- Run via
yarn serve:ssr
- Attempt to access the homepage. The page will never load.
The e2e tests that are run by this PR show a similar behavior.
@tdonohue: I merged the latest |
@alexandrevryghem : This appears to be failing specs at this time. Once that's resolved, I can retest. Here's the error:
|
2c6d04f
to
da320b5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 Thanks @alexandrevryghem ! This now appears to be working properly. I tested it via the custom
theme and dspace
theme, and both appear to be working.
Backport failed for Please cherry-pick the changes locally and resolve any conflicts. git fetch origin dspace-7_x
git worktree add -d .worktree/backport-2651-to-dspace-7_x origin/dspace-7_x
cd .worktree/backport-2651-to-dspace-7_x
git switch --create backport-2651-to-dspace-7_x
git cherry-pick -x d00fc5d9d300ae85cdb3ae82271650d04d48d774 04cf1419eaaa440e94927687bc54154f0dcd6aa5 da320b5fbc52fdc2facc67a55cbd129ed423545e |
It looks like the port to 7.x failed. If we want to port this to 7.x, that'd need to be in a separate PR. That said, I'm OK with this also just being an 8.0 change. |
References
Description
The
ItemListElementComponent
previously used theItemSearchResultListElementComponent
selector directly in its HTML, instead of using theListableObjectComponentLoaderComponent
. Because of this thebase
theme is always used instead of the correct one.Instructions for Reviewers
List of changes in this PR:
ItemListElementComponent
to use theListableObjectComponentLoaderComponent
.ItemListElementComponent#transformItemToItemSearchResult
to convert anItem
to anItemSearchResult
.Guidance for how to test or review this PR:
ItemSearchResultListElementComponent
in your theme (or use thecustom
theme, but don't forget to uncomment the theme insrc/themes/eager-themes.module.ts
)ItemSearchResultListElementComponent
in the browse by pages is now also correctly themedChecklist
yarn lint
yarn check-circ-deps
)package.json
), I've made sure their licenses align with the DSpace BSD License based on the Licensing of Contributions documentation.