Skip to content
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

Enabling navbar links in import section #3220

Merged
merged 2 commits into from
Aug 1, 2024

Conversation

Andrea-Guevara
Copy link
Contributor

References

Description

The “position-fixed” bootstrap class in the html of the “ds-file-dropzone-no-uploader” component was getting in the way of the navbar links.

Instructions for Reviewers

List of changes in this PR:

  • The bootstrap class “position-fixed” on lines 2 and 9 in the html of the “ds-file-dropzone-no-uploader” component has been removed.

To reproduce:

  1. Log in to DSpace as an administrator.
  2. Reveal the left-hand sidebar menu, click Import and then click either Metadata or Batch Import (ZIP).
  3. See that the navigation links in the header are available again.

@kshepherd kshepherd added quick win Pull request is small in size & should be easy to review and/or merge 1 APPROVAL pull request only requires a single approval to merge ux User Experience related works port to dspace-8_x This PR needs to be ported to `dspace-8_x` branch for next bug-fix release labels Jul 25, 2024
@kshepherd kshepherd self-requested a review July 25, 2024 14:33
Copy link
Member

@kshepherd kshepherd left a comment

Choose a reason for hiding this comment

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

Thanks for this quick PR @Andrea-Guevara - I have just tested it now. While it does allow the navbar links to be clicked, unfortunately it seems to have broken the filedrop zone itself? When I drag a CSV file over, I get the initial "Drop a metadata CSV file to import" area displayed in the zone, but nothing happens when I release the mouse button to drop the file.

(presumably the zone itself is no longer as big / in the same place as before, so maybe it's still somewhere on the page, but not where the user expects).

When I revert the change from this PR I am then able to drop a file (I still see the error I mention in my recent issue #3219 but the file is uploaded and the filename is set properly in the process args).

Could you take another look and see if there is a way to make a simple fix like this that keeps the file drop zone working as before?

…er component don't overlap the navbar and make it inaccessible
@Andrea-Guevara
Copy link
Contributor Author

Good afternoon, I apologize for the delay!

@kshepherd thank you very much for your feedback, you've helped us to better understand the problem. We hope it works this time.

Anything else, we'll be happy to help.

@kshepherd
Copy link
Member

Restarting 18.x tests as the failure looks unrelated to this PR:

  Edit Item > Collection Mapper tab
GET /items/6160810f-1e53-40db-81ef-f6621a727398/edit 200 478.302 ms - 380339
GET /reload/1722372157539?redirect=%2Fentities%2Fpublication%2F6160810f-1e53-40db-81ef-f6621a727398%2Fedit%2Fstatus 200 1222.998 ms - -
GET /entities/publication/6160810f-1e53-40db-81ef-f6621a727398/edit/mapper 200 1941.247 ms - -
    ✓ should pass accessibility tests (10129ms)


  7 passing (2m)
  1 failing

  1) Edit Item > Edit Metadata tab
       should pass accessibility tests:
     AssertionError: 1 accessibility violation was detected: expected 1 to equal 0
      at Context.eval (webpack://dspace-angular/./node_modules/cypress-axe/dist/index.js:100:0)

Copy link
Member

@kshepherd kshepherd left a comment

Choose a reason for hiding this comment

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

+1 tested successfully, changes are good and simple. I can use the drag-drop zone and also the header / navbar handlers. Thanks for the fix @Andrea-Guevara ! Merging immediately to main, with a backport to dspace-8_x

@kshepherd kshepherd merged commit a6e3bc3 into DSpace:main Aug 1, 2024
13 checks passed
@dspace-bot
Copy link
Contributor

Successfully created backport PR for dspace-8_x:

@tdonohue tdonohue modified the milestones: 8.1, 9.0 Aug 1, 2024
@tdonohue tdonohue removed the port to dspace-8_x This PR needs to be ported to `dspace-8_x` branch for next bug-fix release label Aug 1, 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 quick win Pull request is small in size & should be easy to review and/or merge ux User Experience related works
Projects
Development

Successfully merging this pull request may close these issues.

Clicking "Import -> [ANY OPTION]" keeps navbar header disabled
4 participants