-
Notifications
You must be signed in to change notification settings - Fork 438
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 home page issues #2844
Fixed home page issues #2844
Conversation
I thought about 2 additional minor improvements that I can add to this PR if there are no objections:
|
Hi @alexandrevryghem, |
Hi @alexandrevryghem, |
7312ee1
to
024f8d0
Compare
@alexandrevryghem : Would you have time to quickly rebase this on the latest |
@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 : 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? |
…facets are disabled
…when showDiscoverFilters is disabled
fbcacf2
to
228779d
Compare
@tdonohue: Sry I don't know what happend 😬 I cherry-picked the changes back on the latest version of |
228779d
to
b2c14df
Compare
@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! |
Also fixed error that showed a blank page instead of a 404 when disabling EULA/Privacy Policy pages
f0e08e2
to
e10630f
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.
👍 Thank you @alexandrevryghem ! This looks great! I also like the new configuration flag to optionally disable the COAR Notify support page, even if you have COAR Notify enabled.
I tested all the fixes locally and everything seems to be working
References
Description
Some fixes for issues on the home page related to new features merged in the past few weeks:
#2275:
ds-themed-configuration-search-page
Reset filters
button on the home page facets#2681:
/info/coar-notify-support
and created an option to hide that link in the footer like done for EULA & Privacy PolicyOther minor fixes:
/info/end-user-agreement
//info/privacy
)/server/api/eperson/profiles/{userId}
to be called when unauthenticated leading into a/server/api/eperson/profiles/undefined
call/community-list
where the names would not be vertically centred in comparison with the chevron icon and item count badgeInstructions for Reviewers
List of changes in this PR:
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 & 768pxHomePageComponent
and into a separate componentChecklist
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.