only add Display Search Facets to the Homepage

[GauravD2t]

References

Add references/links to any related issues or PRs. These may include:

• Fixes Display Search Facets on all Home, Community, and Collection pages DSpace#8426 (if this fixes an issue ticket)

Description

This PR adds Search filters to the Homepage (only). Search filters for Community/Collection pages was split off into #2732

Enabled by default, but can be disabled via this new configuration in config.yml:

homePage:
  # Enable or disable the Discover filters on the homepage
  showDiscoverFilters: true/false

Checklist

This checklist provides a reminder of what we are going to look for when reviewing your PR. You need not complete this checklist prior to creating your PR (draft PRs are always welcome). If you are unsure about an item in the checklist, don't hesitate to ask. We're here to help!

• 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.

[tdonohue]

Hi @GauravD2t : I gave this another look/test today. It's working, but some of the prior feedback has still not been addressed and tests are failing. Overall, this is looking much better and seems nearly ready... we just need to have an option to disable it.

1. In his review, @artlowel noted that we should provide an option to disable this feature as not all sites will want these facets on the homepage. I had also mentioned that in the original design discussion in this comment on the original ticket. I've copied my comment below:
   • "I also want to note that there are potential performance concerns with putting too much content on the homepage. At the very least, this feature needs to be possible to disable if it exists on the homepage."
   • To fix this issue, I'd recommend adding a new config similar to this:

     # This section already exists in `config.example.yml`
     homePage:
       # Enable or disable the Discover filters on the homepage
       showDiscoverFilters: true/false
     

2. Minor issue: I've noted the lines below where tests are failing and how to fix them.
3. Minor issue: There's a misalignment of code issue that I noted previously in search-filter.component.html. It looks like you can simply undo all changes to that file...as the only changes are to accidentally misalign the code (making the code harder to read)

Overall, this is looking good & I thank you for your hard work here.

[tdonohue]

@GauravD2t : Thanks for your hard work on this! I'm nearly a +1 as I can verify this is working properly. However, currently our automatic accessibility tests are failing on the homepage because the "Discover" header is an <h3> when it expects an <h2> (as there's no <h2> heading above that <h3>). I think we should try to find a way to fix this within this PR if possible. (If somehow this is too difficult, we could also consider temporarily disabling the failing accessibility test -- but I'm hopeful we won't need to do that.)

More information on the accessibility error inline below.

[artlowel]

Thanks @GauravD2t

The new configuration option works for me, but I would still disable the feature by default. I'm not a fan of how it makes the homepage take up the full width of the window either

[tdonohue]

@GauravD2t : Could you change the configuration so that this feature is disabled by default as requested by @artlowel above? Thanks. I'll try to give this another test today to make sure my feedback has been addressed.

[tdonohue]

@GauravD2t : Thanks for the updates. I've given this a test today & all my prior feedback has been addressed. I think the final step is to disable this by default (as @artlowel requested above). I've added some notes inline on where those minor changes are necessary.

After these changes are made, I'll give it a final look & then squash merge it (so that it goes in as one commit instead of 70+). Thanks!

[tdonohue]

👍 Thanks @GauravD2t ! This all looks good to me now. Merging.

Feedback was addressed. Disabled by default.