Skip to content

Make ItemSearchResultListElementComponent themeable. - #2828

Closed
mwoodiupui wants to merge 2 commits into
DSpace:mainfrom
mwoodiupui:themed-item-search
Closed

Make ItemSearchResultListElementComponent themeable.#2828
mwoodiupui wants to merge 2 commits into
DSpace:mainfrom
mwoodiupui:themed-item-search

Conversation

@mwoodiupui

Copy link
Copy Markdown
Member

Description

Make ItemSearchResultListElementComponent themeable.

Instructions for Reviewers

List of changes in this PR:

  • Create ThemedItemSearchResultListElementComponent.
  • Add it to SharedModule.
  • Update references of ItemSearchResultListElementComponent

Include guidance for how to test or review your PR. This may include: steps to reproduce a bug, screenshots or description of a new feature, or reasons behind specific changes.

Checklist

  • My PR is small in size (e.g. less than 1,000 lines of code, not including comments & specs/tests), or I have provided reasons as to why that's not possible.
  • My PR passes ESLint validation using yarn lint
  • My PR doesn't introduce circular dependencies (verified via yarn check-circ-deps)
  • My PR includes TypeDoc comments for all new (or modified) public methods and classes. It also includes TypeDoc for large or complex private methods.
  • My PR passes all specs/tests and includes new/updated specs or tests based on the Code Testing Guide.
  • If my PR includes new libraries/dependencies (in package.json), I've made sure their licenses align with the DSpace BSD License based on the Licensing of Contributions documentation.
  • If my PR includes new features or configurations, I've provided basic technical documentation in the PR itself.
  • If my PR fixes an issue ticket, I've linked them together.

@mwoodiupui

Copy link
Copy Markdown
Member Author

WIP because, while this met my immediate need, I discovered a number of additional references to this component which I need to update.

@toniprieto

Copy link
Copy Markdown
Contributor

@mwoodiupui Does it solve the same issue? #2651

@github-actions

github-actions Bot commented Mar 8, 2024

Copy link
Copy Markdown

Hi @mwoodiupui,
Conflicts have been detected against the base branch.
Please resolve these conflicts as soon as you can. Thanks!

@github-actions

Copy link
Copy Markdown

Hi @mwoodiupui,
Conflicts have been detected against the base branch.
Please resolve these conflicts as soon as you can. Thanks!

@alexandrevryghem

Copy link
Copy Markdown
Member

I'm closing this PR since it's a duplicate of #2651. We should normally never create themed- versions of dynamic components nor use their selectors in the html files directly. Instead we should use their dynamic component loaders instead. That way it can be themed like the other dynamic components (by proving the correct theme in the decorator as the last parameter)

@mwoodiupui

Copy link
Copy Markdown
Member Author

I'm happy to see this fixed.

I would have tried to do this the right way if I had known:

  • how to know by looking at it that ItemSearchResultListElementComponent is a "dynamic component"
  • what's a dynamic component?
  • where to find the documentation on how we theme dynamic components

It seems to me that the theming support is still opaque in some areas.

@mwoodiupui
mwoodiupui deleted the themed-item-search branch April 15, 2024 12:28
@alexandrevryghem

Copy link
Copy Markdown
Member

Previously you could easily see that a component was dynamically themeable because they have a custom decorator on top of the component for example @listableObjectComponent (this is the only one that is still used on dspace-8.0-next). Starting from dspace-8.0-next you can still find out if it's a dynamic component if it is defined in one of de the decorator files for example here (info about why this was migrated)

An example on how to theme these can be found here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants