Skip to content

[CST-5341] Link back to the item page in the now downloading page#1530

Merged
tdonohue merged 2 commits intoDSpace:mainfrom
4Science:CST-5341-back-button
Feb 23, 2022
Merged

[CST-5341] Link back to the item page in the now downloading page#1530
tdonohue merged 2 commits intoDSpace:mainfrom
4Science:CST-5341-back-button

Conversation

@davide-negretti
Copy link
Copy Markdown
Contributor

References

Description

A "Back" button has been added to the download page

Instructions for Reviewers

Open any item with an attached file, e.g. this one, and download the file. The download page should have a "Back" button.

  • 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 TSLint validation using yarn run lint
  • My PR doesn't introduce circular dependencies
  • 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, third-party dependencies (in package.json), I've made sure their licenses align with the DSpace BSD License based on the Licensing of Contributions documentation.

@tdonohue tdonohue added 1 APPROVAL pull request only requires a single approval to merge bug labels Feb 17, 2022
@tdonohue tdonohue added this to the 7.3 milestone Feb 17, 2022
@tdonohue tdonohue self-requested a review February 17, 2022 16:03
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.

@davidenegretti-4science : I tested this today. The code looks good overall, and the button works.

However, I feel the "Back" button placement is confusing. It's not obvious placement as it appears over under the Log In menu (and I admit I didn't even notice it at first glance...so I think it's a minor usability issue). I think we should move it to just below the "Now downloading" text. (See the screenshot below... it should be moved to the location I circled in red)
back-button

So, overall, I'm +1. However, I think this minor change will improve usability of this feature.

@davide-negretti
Copy link
Copy Markdown
Contributor Author

Hi @tdonohue, I changed the position of the button

Schermata del 2022-02-18 10-48-58

@tdonohue tdonohue self-requested a review February 18, 2022 16:14
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 @davidenegretti-4science ! I've retested this today and it now looks good to me.

@tdonohue tdonohue merged commit cba0ad6 into DSpace:main Feb 23, 2022
@abollini abollini deleted the CST-5341-back-button branch May 1, 2022 10:43
4science-it pushed a commit to 4Science/dspace-angular that referenced this pull request Apr 24, 2024
[DSC-1590]

Approved-by: Andrea Barbasso
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 bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Back button is missing from download page

2 participants