Skip to content

Fixed item-edit.cy.ts regularly failing & minor UI improvements#3056

Merged
tdonohue merged 7 commits intoDSpace:mainfrom
alexandrevryghem:minor-ui-improvements_contribute-main
May 16, 2024
Merged

Fixed item-edit.cy.ts regularly failing & minor UI improvements#3056
tdonohue merged 7 commits intoDSpace:mainfrom
alexandrevryghem:minor-ui-improvements_contribute-main

Conversation

@alexandrevryghem
Copy link
Copy Markdown
Member

References

Description

Fixed the item-edit.cy.ts regularly failing because sometimes the <div role="row"> in the <ds-dso-edit-metadata-value> doesn't contain children with role="cell" when the accessibility checks are run. This PR also contains some other minor UI improvements.

Instructions for Reviewers

List of changes in this PR:

  • Fixed the item-edit.cy.ts test, to prove this I ran the test with the fix 50 times in the CICD and it doesn't fail anymore.
  • Fixed UI issue where the tooltip was sometimes rendered behind the header based on it's z-index and when there is a lot of text. To fix this I added the container="body" to the HTML tag with the tooltip.
  • Fixed COAR link on /info/coar-notify-support not working and replaced the logo with an SVG to save a few kb 😅
  • On screens smaller than 768px the ds-community-search-result-list-element is not aligning like collections and communities
    image
  • Fixed <ds-my-dspace-qa-events-notifications>'s logo sometimes overflowing out of the notification
    image
  • Accessibility: Added missing roles to comcol tabs & added alt attribute to the DSpace logo on the logout page
  • Replaced the PNG logos of the login & logout pages with the SVG version in order to retrieve one less asset

Checklist

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

@alexandrevryghem alexandrevryghem self-assigned this May 15, 2024
@alexandrevryghem alexandrevryghem added bug accessibility testathon Reported by a tester during Community Testathon testing framework Related specifically to Unit or Integration (e2e) Tests integration: COAR Notify / LDN Related to Linked Data Notifications (LDN) or COAR Notify services labels May 15, 2024
@tdonohue tdonohue added this to the 8.0 milestone May 16, 2024
@tdonohue tdonohue self-requested a review May 16, 2024 11:49
@tdonohue tdonohue added the 1 APPROVAL pull request only requires a single approval to merge label May 16, 2024
Copy link
Copy Markdown
Member

@tdonohue tdonohue left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Thanks @alexandrevryghem ! This all looks great. I've tested all the various changes mentioned in the description and they all appear to be working as described. Also verified linked issue is fixed.

@tdonohue tdonohue merged commit d74e672 into DSpace:main May 16, 2024
@alexandrevryghem alexandrevryghem deleted the minor-ui-improvements_contribute-main branch May 16, 2024 18:25
@tdonohue
Copy link
Copy Markdown
Member

@alexandrevryghem : I'm just now realizing that this probably should be partially ported to 7.x. The same item-edit.cy.ts issues exist there, along with some of these minor UI issues.

I'm going to flag this as port to dspace-7_x until one of us gets to it. There's no rush (the next 7.x release won't go out until after 8.0 is released), but I don't want to forget this. If you don't find time, I might be able to eventually help port this one.

@tdonohue tdonohue added the port to dspace-7_x This PR needs to be ported to `dspace-7_x` branch for next bug-fix release label May 17, 2024
@alexandrevryghem alexandrevryghem removed the port to dspace-7_x This PR needs to be ported to `dspace-7_x` branch for next bug-fix release label May 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

1 APPROVAL pull request only requires a single approval to merge accessibility bug integration: COAR Notify / LDN Related to Linked Data Notifications (LDN) or COAR Notify services testathon Reported by a tester during Community Testathon testing framework Related specifically to Unit or Integration (e2e) Tests

Projects

No open projects
Status: ✅ Done

Development

Successfully merging this pull request may close these issues.

COAR Notify website link on COAR Notify Support page doesn't work (v8 testathon)

2 participants