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

document: display cover thumbnail #438

Conversation

AoNoOokami
Copy link
Contributor

@AoNoOokami AoNoOokami commented Nov 18, 2020

Co-Authored-by: Alicia Zangger alicia.zangger@rero.ch

Why are you opening this PR?

To display thumbnails in brief and detail views of both admin and public parts of the application.
This is a part of US1812.

Dependencies

My PR depends on rero-ils#<xx>'s PR(s):

N/A

How to test?

Checkout this PR locally.
npm i to install shared library
npm run start-admin-proxy to check admin part
npm run start-public-search-proxy to check public part

Check thumbnail display in both projects and both views (brief and detailed) on localhost:4200.

Code review check list

  • Commit message template compliance.
  • Commit message without typos.
  • File names.
  • Functions names.
  • Functions docstrings.
  • Unnecessary commited files?

@AoNoOokami AoNoOokami changed the base branch from dev to zaa-US1812-metadata-document-types November 18, 2020 15:20
@AoNoOokami AoNoOokami force-pushed the zaa-#1867-document-types branch 2 times, most recently from fdb748e to 49c663c Compare November 18, 2020 15:36
@AoNoOokami AoNoOokami marked this pull request as draft November 18, 2020 15:40
@AoNoOokami AoNoOokami force-pushed the zaa-#1867-document-types branch 2 times, most recently from a0459ae to a35bd56 Compare November 18, 2020 16:20
@AoNoOokami AoNoOokami changed the title document: add document types in brief and detailed views document: display thumbnail Nov 18, 2020
@AoNoOokami AoNoOokami marked this pull request as ready for review November 18, 2020 16:21
@AoNoOokami AoNoOokami self-assigned this Nov 18, 2020
@AoNoOokami AoNoOokami added this to the v0.15.0 milestone Nov 18, 2020
Copy link
Contributor

@zannkukai zannkukai left a comment

Choose a reason for hiding this comment

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

Some comment are cosmetics ;-)

for (const identifier of this.record.metadata.identifiedBy) {
if (identifier.type === 'bf:Isbn') {
this.isbn = identifier.value;
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

this.isbn = this.record.metadata.identifyBy
    .filter(id => id.type === 'bf:Isbn')  // keep only isbn
    .map(id => id.value)  // get identifier value
    .find()  // return the first array value or undefined if array is empty

To be tested in real situation. For my point of view, it's more javascript minded (and if I had a good memory, it's more efficient for CPU)

<header class="col-10">
<div class="row mt-3 mb-3">
<div class="col-2">
<shared-thumbnail [record]="record"></shared-thumbnail>
Copy link
Contributor

Choose a reason for hiding this comment

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

Added the class parameter to be more flexible. The CSS definition stays in the main application.

Copy link
Contributor

@iGormilhit iGormilhit left a comment

Choose a reason for hiding this comment

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

Is it cover thumbnail? If so, the commit title could be amended.

Copy link
Contributor

@iGormilhit iGormilhit left a comment

Choose a reason for hiding this comment

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

I think you're closing rero/rero-ils#1396

@iGormilhit iGormilhit dismissed their stale review November 19, 2020 08:21

I was wrong.

@AoNoOokami AoNoOokami force-pushed the zaa-#1867-document-types branch from a35bd56 to d396d5b Compare November 20, 2020 08:02
@AoNoOokami AoNoOokami force-pushed the zaa-#1867-document-types branch 2 times, most recently from f1167d4 to d31713a Compare November 20, 2020 08:24
@AoNoOokami AoNoOokami changed the title document: display thumbnail document: display cover thumbnail Nov 20, 2020
@AoNoOokami AoNoOokami force-pushed the zaa-#1867-document-types branch 2 times, most recently from 7b435c5 to 9b7994c Compare November 20, 2020 10:40
Copy link
Contributor

@iGormilhit iGormilhit left a comment

Choose a reason for hiding this comment

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

Commit message approved.

@AoNoOokami AoNoOokami force-pushed the zaa-#1867-document-types branch from 9b7994c to 34e1b07 Compare November 25, 2020 14:30
@AoNoOokami AoNoOokami force-pushed the zaa-#1867-document-types branch from 34e1b07 to 118659a Compare December 1, 2020 14:38
* Moves thumbnail logic in 'shared' library as it's used in both admin
and public-search projects.
* Adds thumbnails in professional brief and detailed view.
* Moves 'type' field below the thumbnail in admin detailed view.
* Closes rero/rero-ils#1188.

Co-Authored-by: Alicia Zangger <alicia.zangger@rero.ch>
@AoNoOokami AoNoOokami force-pushed the zaa-#1867-document-types branch from 118659a to 5190a04 Compare December 1, 2020 14:38
@AoNoOokami AoNoOokami merged commit 1a58755 into rero:zaa-US1812-metadata-document-types Dec 4, 2020
@iGormilhit iGormilhit added the f: professional ui Professional interface label Dec 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
f: professional ui Professional interface
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants