dspace-angular icon indicating copy to clipboard operation
dspace-angular copied to clipboard

Make ItemSearchResultListElementComponent themeable.

Open mwoodiupui opened this issue 1 year ago • 4 comments

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

  • [x] 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.
  • [x] My PR passes ESLint validation using yarn lint
  • [x] 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.
  • [x] 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.
  • [x] If my PR includes new features or configurations, I've provided basic technical documentation in the PR itself.
  • [x] If my PR fixes an issue ticket, I've linked them together.

mwoodiupui avatar Feb 22 '24 14:02 mwoodiupui

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

mwoodiupui avatar Feb 22 '24 14:02 mwoodiupui

@mwoodiupui Does it solve the same issue? https://github.com/DSpace/dspace-angular/pull/2651

toniprieto avatar Feb 22 '24 16:02 toniprieto

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

github-actions[bot] avatar Mar 08 '24 15:03 github-actions[bot]

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

github-actions[bot] avatar Mar 20 '24 14:03 github-actions[bot]

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)

alexandrevryghem avatar Apr 13 '24 17:04 alexandrevryghem

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 avatar Apr 15 '24 12:04 mwoodiupui

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

alexandrevryghem avatar Apr 15 '24 12:04 alexandrevryghem