CST-12044 visualize the primary bitstream & CST-12043 primary bitstream flag #2631

vNovski · 2023-11-13T08:51:35Z

Please note that this development has been funded by the University of California - California Digital Library.

References

DSpace PR: DSpace/DSpace#9195

Description

Added a bitstream switch on the download page for each file and also add a switch in the modal editing window of a specific file, which can be used to make the bitstream the primary.
Added a badge primary aside the file name for the bitstream that has been eventually set as primary

INTERNAL DOCUMENTATION:

Added a new method to bitstream-data.service.ts findPrimaryBitstreamByItemAndName() which allows you to get a primary bitstream based on item and origin
In addition to implementing the business logic described in the task, the behavior of the custom switch was also fixed so that it was correctly processed by the form builder, specifically data normalization. Also, the custom switch did not use translate pipe for automatic label translation

Instructions for Reviewers

Review for primary bitstream switch has been done with the following cases:

Edit Submission form
- Upload files:
  - Scroll down to file list
    - activate the switch for one of the files - automaticaly saves and updates
    - activate the switch for another file → previous primary bitstream turns off automatically and new primary bitstream turns on then automaticaly saves and updates
    - deactivate the switch of current primary bitstream → primary bitstream turns off then automaticaly saves and updates
    - activate the switch for one of the files - automaticaly saves and updates
    - delete file witch is primary - file is deleted and primary bitstream is reseted then automaticaly saves and updates
  - open file edit modal for one of the files
    - activate the primary bitstream switcher then click save - saves and updates
  - open file edit modal for file witch is not primary
    - activate the primary bitstream switcher then click save - previous primary bitstream turns off automatically and new primary bitstream turns on then saves and updates
  - open file edit modal for file witch is primary
    - deactivate the switch of primary bitstream then click save → primary bitstream turns off then saves and updates

SCREENSHOTS:
Modal:

Upload Files form:

Review for badge which indicates primary bitstream has been done with the following cases:

if bitstream is primary then badge [Primary] is displayed
if bitstream is not primary then badge [Primary] is not displayed

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.

…3-primary-bitstream-flag

…ize-the-primary-bitstream

…eam-flag' into CST-12043-primary-bitstream-flag

…ize-the-primary-bitstream

hardyoyo · 2024-01-22T18:55:59Z

I'm attempting to test this PR in concert with the backend PR... and... I cannot see the primary bitstream switch on the submission form. Is there any chance you can post a screenshot of what this is supposed to look like for the submission form (not the edit a bitstream modal, I'm aware of what that modal looks like, and that functionality still works in my testing).

I'm attaching a screenshot of what I see on the upload form:

tdonohue

@vNovski : Thanks! Tested and reviewed this today. It looks good overall & works! That said, I found a small bug, and I think we may want to look at the layout of the page a bit.

Here's what I see on the submission form with this PR (and the backend PR):

A few points about that layout:

The "Primary Bitstream" header is aligned right, while the buttons are aligned center. It makes it look a bit odd.
I think we need a header over the file section that says something like "File Details" or "File Information". That would make it clear this is a table of information.
(Not Required) I'm starting to wonder if we should rename the "Primary Bitstream" header on the submission form to say "Primary File" or "Primary?". I noticed that submission form doesn't use the term "bitstsream". So, we may want to consider just calling everything a "file" on this page.

One small bug that I noticed: If you click on a primary bitstream, then accept/reject the license, then change the primary bitstream the changes are not saved. Here's how to reproduce:

Upload at least to bitstreams on the submission form
Select one as primary (should succeed)
Now either check or uncheck the license checkbox
Immediately select the other bitstream as the primary bitstream.
No error will occur, but the primary bitstream is unchanged. It appears the PATCH request sent was only to update the license checkbox and NOT for the new primary bitstream.

I have a few other minor comments inline. Overall though this all looks good.

package.json

src/app/submission/sections/upload/file/section-upload-file.component.html

src/app/submission/sections/upload/section-upload.component.html

vNovski · 2024-02-06T14:37:09Z

@vNovski : Thanks! Tested and reviewed this today. It looks good overall & works! That said, I found a small bug, and I think we may want to look at the layout of the page a bit.

Here's what I see on the submission form with this PR (and the backend PR):

A few points about that layout:

The "Primary Bitstream" header is aligned right, while the buttons are aligned center. It makes it look a bit odd.

I think we need a header over the file section that says something like "File Details" or "File Information". That would make it clear this is a table of information.

(Not Required) I'm starting to wonder if we should rename the "Primary Bitstream" header on the submission form to say "Primary File" or "Primary?". I noticed that submission form doesn't use the term "bitstsream". So, we may want to consider just calling everything a "file" on this page.

One small bug that I noticed: If you click on a primary bitstream, then accept/reject the license, then change the primary bitstream the changes are not saved. Here's how to reproduce:

Upload at least to bitstreams on the submission form

Select one as primary (should succeed)

Now either check or uncheck the license checkbox

Immediately select the other bitstream as the primary bitstream.

No error will occur, but the primary bitstream is unchanged. It appears the PATCH request sent was only to update the license checkbox and NOT for the new primary bitstream.

I have a few other minor comments inline. Overall though this all looks good.

For the pull request, I made the following changes:

Removed --code-coverage from package.json.
Added two texts for the tag, depending on the selected option: "Make {{fileName}} the primary bitstream" and "Remove {{fileName}} as the primary bitstream".
Fixed linting issues.
Changed the label "Primary bitstream" to "Primary File".
Fixed an issue with the license button that was preventing saving, now everything works correctly.

tdonohue

Thanks @vNovski ! This looks good except for a very minor issue. I have retested & verified that all prior bugs are fixed and everything else looks good.

So, once this issue is fixed (and the backend PR is ready), then this will be ready to merge.

tdonohue · 2024-02-14T22:53:20Z

src/app/submission/sections/upload/file/section-upload-file.component.html

+        [checked]="isPrimary" 
+        (change)="togglePrimaryBitstream($event)">
+        <label class="custom-control-label" for="primaryBitstream{{fileIndex}}">
+          <span class="sr-only" aria-hidden="true" *ngIf="!isPrimary">{{'submission.sections.upload.primary.make' | translate:{ fileName: fileName } }}</span>


aria-hidden=true should be removed from this <span> along with the one below it.

I think I mistakenly asked you to add this.. My apologies as I was wrong. The class="sr-only" is all that is needed to pass accessibility checks. Currently, the accessibility checks in "axe DevTools" fail on this line because the aria-hidden=true hides the text in the span from the accessibility scanner.

tdonohue · 2024-02-14T23:00:50Z

src/app/submission/sections/upload/file/section-upload-file.component.html

+        (change)="togglePrimaryBitstream($event)">
+        <label class="custom-control-label" for="primaryBitstream{{fileIndex}}">
+          <span class="sr-only" aria-hidden="true" *ngIf="!isPrimary">{{'submission.sections.upload.primary.make' | translate:{ fileName: fileName } }}</span>
+          <span class="sr-only" aria-hidden="true" *ngIf="isPrimary">{{'submission.sections.upload.primary.remove' | translate:{ fileName: fileName } }}</span>


Same here, please remove aria-hidden=true attribute

vNovski · 2024-02-15T16:36:29Z

@vNovski : Thanks! Tested and reviewed this today. It looks good overall & works! That said, I found a small bug, and I think we may want to look at the layout of the page a bit.
Here's what I see on the submission form with this PR (and the backend PR):
A few points about that layout:

The "Primary Bitstream" header is aligned right, while the buttons are aligned center. It makes it look a bit odd.

I think we need a header over the file section that says something like "File Details" or "File Information". That would make it clear this is a table of information.

(Not Required) I'm starting to wonder if we should rename the "Primary Bitstream" header on the submission form to say "Primary File" or "Primary?". I noticed that submission form doesn't use the term "bitstsream". So, we may want to consider just calling everything a "file" on this page.

One small bug that I noticed: If you click on a primary bitstream, then accept/reject the license, then change the primary bitstream the changes are not saved. Here's how to reproduce:

Upload at least to bitstreams on the submission form

Select one as primary (should succeed)

Now either check or uncheck the license checkbox

Immediately select the other bitstream as the primary bitstream.

No error will occur, but the primary bitstream is unchanged. It appears the PATCH request sent was only to update the license checkbox and NOT for the new primary bitstream.

I have a few other minor comments inline. Overall though this all looks good.

For the pull request, I made the following changes:

Removed --code-coverage from package.json.

Added two texts for the tag, depending on the selected option: "Make {{fileName}} the primary bitstream" and "Remove {{fileName}} as the primary bitstream".

Fixed linting issues.

Changed the label "Primary bitstream" to "Primary File".

Fixed an issue with the license button that was preventing saving, now everything works correctly.

For the pull request, I made the following changes:

The aria-hidden attribute has been removed.

tdonohue

👍 Thanks @vNovski !

tdonohue · 2024-02-20T23:12:45Z

While this has been merged, I'm flagging as needs documentation until we can update the Submission documentation to include basic details about this feature: https://wiki.lyrasis.org/display/DSDOC8x/Submission+User+Interface

GraziaQuercia · 2024-03-01T17:12:54Z

Hi @tdonohue here's the documentation page: https://wiki.lyrasis.org/display/DSDOC8x/Set+a+bitstream+as+primary
If any change is needed, let me know! Thanks in advance

vNovski and others added 21 commits October 20, 2023 16:04

[CST-12043] fix: normalization for boolean

a55eb97

[CST-12043] feature: add primary bitstream switch

ffdeca6

[CST-12043] unit tests

bbc8b6a

Merge commit 'bbc8b6ac8a15bdd619d4f7dac90655ab52dbad2c' into CST-1204…

1632795

…3-primary-bitstream-flag

[CST-12043] fix: translation for custom switch

ae93cc0

[CST-12043] ref: add and update documentation

6fc75a5

[CST-12043] fix: redundant space

7c723ca

[CST-12043] fix: dependencies for unit test

f006bb3

[CST-12044] feature: badge for primary bitstream

7cee04d

[CST-12044] feature: add code coverage flag

c644cc5

[CST-12044] feature: add translation for badge

462aafb

Merged cdl7 into CST-12043-primary-bitstream-flag

027a6b0

Merge branch 'CST-12043-primary-bitstream-flag' into CST-12044-visual…

9322129

…ize-the-primary-bitstream

[CST-12043] feature: add title

ca64bac

Merge remote-tracking branch '4cience-origin/CST-12043-primary-bitstr…

823f728

…eam-flag' into CST-12043-primary-bitstream-flag

[CST-12043] feature: add label position

0f1f4e8

[CST-12043] Add class for element container

41eb34b

[CST-12043] Remove ng-deep styles

034d7dd

[CST-12043] Fix layout

9ce9606

Merge branch 'CST-12043-primary-bitstream-flag' into CST-12044-visual…

f742039

…ize-the-primary-bitstream

[CST-12044] refactor: styles

71af428

github-actions bot assigned vNovski Nov 13, 2023

tdonohue mentioned this pull request Nov 13, 2023

Add support for the primary bitstream flag DSpace/DSpace#9195

Merged

8 tasks

tdonohue added component: submission new feature labels Nov 13, 2023

tdonohue added this to the 8.0 milestone Nov 13, 2023

vNovski marked this pull request as ready for review November 23, 2023 09:14

tdonohue requested review from tdonohue and hardyoyo December 14, 2023 15:59

tdonohue requested changes Jan 30, 2024

View reviewed changes

vNovski added 2 commits February 6, 2024 13:23

[CST-12044] refactor: code

b865259

[CST-12044] fix accept/reject the license bug

18e62f4

tdonohue self-requested a review February 6, 2024 15:29

[CST-12044] refactor: code

c596b6c

tdonohue reviewed Feb 14, 2024

View reviewed changes

[CST-12044] removed aria hidden attribute

02baf86

tdonohue approved these changes Feb 20, 2024

View reviewed changes

tdonohue merged commit dcf5836 into DSpace:main Feb 20, 2024
13 checks passed

tdonohue mentioned this pull request Feb 20, 2024

Add primary bitstream support to the submission process DSpace/DSpace#8834

Closed

tdonohue added the needs documentation PR is missing documentation. All new features and config changes require documentation. label Feb 20, 2024

tdonohue removed the needs documentation PR is missing documentation. All new features and config changes require documentation. label May 31, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CST-12044 visualize the primary bitstream & CST-12043 primary bitstream flag #2631

CST-12044 visualize the primary bitstream & CST-12043 primary bitstream flag #2631

vNovski commented Nov 13, 2023 •

edited

Loading

hardyoyo commented Jan 22, 2024 •

edited

Loading

tdonohue left a comment

vNovski commented Feb 6, 2024

tdonohue left a comment

tdonohue Feb 14, 2024 •

edited

Loading

tdonohue Feb 14, 2024

vNovski commented Feb 15, 2024

tdonohue left a comment

tdonohue commented Feb 20, 2024

GraziaQuercia commented Mar 1, 2024

CST-12044 visualize the primary bitstream & CST-12043 primary bitstream flag #2631

CST-12044 visualize the primary bitstream & CST-12043 primary bitstream flag #2631

Conversation

vNovski commented Nov 13, 2023 • edited Loading

References

Description

Instructions for Reviewers

Checklist

hardyoyo commented Jan 22, 2024 • edited Loading

tdonohue left a comment

Choose a reason for hiding this comment

vNovski commented Feb 6, 2024

tdonohue left a comment

Choose a reason for hiding this comment

tdonohue Feb 14, 2024 • edited Loading

Choose a reason for hiding this comment

tdonohue Feb 14, 2024

Choose a reason for hiding this comment

vNovski commented Feb 15, 2024

tdonohue left a comment

Choose a reason for hiding this comment

tdonohue commented Feb 20, 2024

GraziaQuercia commented Mar 1, 2024

vNovski commented Nov 13, 2023 •

edited

Loading

hardyoyo commented Jan 22, 2024 •

edited

Loading

tdonohue Feb 14, 2024 •

edited

Loading