-
Notifications
You must be signed in to change notification settings - Fork 437
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
Made AdminSearchPageComponent themeable #3065
Made AdminSearchPageComponent themeable #3065
Conversation
…o new-themed-components_contribute-main # Conflicts: # src/app/admin/admin-routing.module.ts # src/app/admin/admin-search-page/admin-search.module.ts # src/themes/custom/lazy-theme.module.ts
…tribute-7.4' into new-themed-components_contribute-main # Conflicts: # src/app/admin/admin-routing.module.ts # src/app/admin/admin-workflow-page/admin-workflow.module.ts # src/themes/custom/lazy-theme.module.ts
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.
Thanks @alexandrevryghem! I tested this and confirmed that I can add Admin Search and Workflow to a custom theme. And it fixes an obvious bug with the MetadataService. I just had one little change to update the function comment.
Also fixed minor issue in MetadataService, but this doesn't cause any issues in the current code
13e6bed
to
446280b
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.
👍 Looks good!
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.
👍 Thanks @alexandrevryghem ! Verified that this also fixes the missing focus indicator accessibility bug. Code looks good. Because this includes an accessibility fix, I'm marking this to be ported to 8.x & 7.x. If the automated ports don't succeed, we'll want to minimally port the accessibility fix to those releases.
Backport failed for Please cherry-pick the changes locally and resolve any conflicts. git fetch origin dspace-7_x
git worktree add -d .worktree/backport-3065-to-dspace-7_x origin/dspace-7_x
cd .worktree/backport-3065-to-dspace-7_x
git switch --create backport-3065-to-dspace-7_x
git cherry-pick -x 99384a70605b2eccaaae249103673c3735da8c85 61e52cd2abb8ec5dea4b17170e93fe190b086dc9 970b19bf0104a9f361717d148bfc66a60e70e239 446280b59a65c8c0ce24a796ffd14b5eef6eafd0 |
Backport failed for Please cherry-pick the changes locally and resolve any conflicts. git fetch origin dspace-8_x
git worktree add -d .worktree/backport-3065-to-dspace-8_x origin/dspace-8_x
cd .worktree/backport-3065-to-dspace-8_x
git switch --create backport-3065-to-dspace-8_x
git cherry-pick -x 99384a70605b2eccaaae249103673c3735da8c85 61e52cd2abb8ec5dea4b17170e93fe190b086dc9 970b19bf0104a9f361717d148bfc66a60e70e239 446280b59a65c8c0ce24a796ffd14b5eef6eafd0 |
@alexandrevryghem : Could you manually port either this entire PR, or just the focus indicator fix to 8.x and 7.x? Looks like this cannot be ported automatically. |
References
Description
Made
AdminSearchPageComponent
andAdminWorkflowPageComponent
themeable & fixed minor css issue where the navbar would overflow over the breadcrumbs in mobile modeChecklist
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.