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

Fixed home page issues

Open alexandrevryghem opened this issue 1 year ago • 3 comments

References

  • Small fixes related to #2275
  • Small fixes related to #2681

Description

Some fixes for issue on the home page related to new features merged in the past weeks:

  • The home page content (communities in dspace & recent submissions) is not alligned anymore with the home news content
  • For screens smaller than 576 pixels the home page facets are being displayed on top of the communities section & recent submission section
  • The home page used the non themed version of ds-themed-configuration-search-page
  • ~~The error ERROR TypeError: You provided 'undefined' where a stream was expected. You can provide an Observable, Promise, ReadableStream, Array, AsyncIterable, or Iterable. is currently being thrown on https://sandbox.dspace.org/home (only happens when COAR is disabled)~~
  • Fix minor memory leak on home page (subscription not being cleaned up)
  • Prevent /server/api/eperson/profiles/{userId} to be called when unauthenticated leading into a /server/api/eperson/profiles/undefined call

Instructions for Reviewers

List of changes in this PR:

  • Rewrote the way the search facets are being rendered next to the home page content. I used the PageWithSidebarComponent to render them, this has some benefits for example on screen with a small height the facets bar will move along with you and won't stay at the top on long pages. It will also automatically use the whole width to display the search filters when your screen is between 576px & 768px
  • Display the expand collapse button of search facets on screens between 576px & 768px above the search bar
  • Fixed the console error when COAR was disabled by adding a return value in the switchMap when coar is disabled

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)
  • [x] My PR includes TypeDoc comments for all new (or modified) public methods and classes. It also includes TypeDoc for large or complex private methods.
  • [x] 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.

alexandrevryghem avatar Feb 29 '24 23:02 alexandrevryghem

I thought about 2 additional minor improvements that I can add to this PR if there are no objections:

  • We could maybe hide the Reset filters button when inPlaceSearch is false, because it simply redirect you to the search page, and you can't select any filter on the home page (you are automatically redirected to the search page).
  • We could also maybe move the COAR code from the home page to a dedicated component. We can still call that component in the HTML of HomePageComponent, but this way we clearly seperate that functionality from the home page code

alexandrevryghem avatar Mar 01 '24 00:03 alexandrevryghem

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

github-actions[bot] avatar Mar 04 '24 22:03 github-actions[bot]

Hi @alexandrevryghem, 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]

@alexandrevryghem : Would you have time to quickly rebase this on the latest main? I want to make sure this is compatible with the recent updates to Angular 16 (I suspect it will be, but currently it's not running on the latest code in tests)

tdonohue avatar Apr 10 '24 18:04 tdonohue

@tdonohue: I retested it locally after merging the latest main and fixed a remaining spacing issue on the home page and also already implemented these suggestions

alexandrevryghem avatar Apr 10 '24 19:04 alexandrevryghem

@alexandrevryghem : it appears this PR accidentally includes a ton of unrelated commits now (it shows over 8,000 lines of code changed). Could you try to clean it up quickly so that I can test it today/tomorrow?

tdonohue avatar Apr 10 '24 20:04 tdonohue

@tdonohue: Sry I don't know what happend 😬 I cherry-picked the changes back on the latest version of main

alexandrevryghem avatar Apr 10 '24 21:04 alexandrevryghem

@alexandrevryghem : No worries! I've been in the same situation... sometimes Git/GitHub does things that are unexplainable. It looks good now & I'll try to review this one tomorrow. Thanks!

tdonohue avatar Apr 10 '24 21:04 tdonohue