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

circulation: display circulation notes automatically. #280

Merged
merged 1 commit into from
Jun 20, 2020

Conversation

zannkukai
Copy link
Contributor

@zannkukai zannkukai commented Jun 10, 2020

When circulation operations are done, if item contains the corresponding
circulation note, this note will be displayed as a permanent toastr
message.

  • refactoring item circulation collapsed informations design.
  • adds postprocessRecord behavior to avoid empty notes array on item.

Co-authored-by : Renaud Michotte renaud.michotte@gmail.com

Why are you opening this PR?

https://tree.taiga.io/project/rero21-reroils/us/1352?milestone=266303

Dependencies

(backend) rero/rero-ils#1026

How to test?

  • Identify some items with checkin/checkout notes OR add checkin/checkout notes for some circulating items
  • Do some checkin/checkout operation on these items.
  • If a checkin/checkout notes are defined on an item, then a permanent toastr message should be display.

image

Code review check list

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

@zannkukai zannkukai force-pushed the zan-#item-notes-circulation-messages branch from 371d9a7 to ce0e73c Compare June 10, 2020 08:09
@zannkukai zannkukai marked this pull request as ready for review June 10, 2020 08:25
@zannkukai zannkukai self-assigned this Jun 10, 2020
@zannkukai zannkukai force-pushed the zan-#item-notes-circulation-messages branch from ce0e73c to b43b055 Compare June 10, 2020 09:16
@zannkukai zannkukai force-pushed the zan-#item-notes-circulation-messages branch from b43b055 to a5ea4ce Compare June 10, 2020 09:58
@zannkukai zannkukai force-pushed the zan-#item-notes-circulation-messages branch from a5ea4ce to 8ed0dcd Compare June 15, 2020 08:50
@pronguen pronguen self-requested a review June 15, 2020 15:54
Copy link
Contributor

@pronguen pronguen left a comment

Choose a reason for hiding this comment

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

Two comments

  1. Should we have an upper case on "Exemplaire" in the checkin and checkout note?
    image
    Actually I would simplify, remove this text and the item barcode, and display only the note content.

  2. If there is a checkin/checkout note, I would add a small icon next to the barcode in the loan line. It is easier to find the relation than to compare barcodes. @iGormilhit This was your proposal during last discussion, right?

@iGormilhit
Copy link
Contributor

iGormilhit commented Jun 16, 2020

@pronguen Yes, as I remember, I proposed this also for the document detailed view (item barcode in the holdings). But, that was to identify item with note. To make the link between the toast message and the item, I don't know if it enough. The barcode is indeed not so easy to remember quickly (I would not, most of the time, but I'm old, you know 😉 ).

@JoelleDosimont
Copy link

1. Should we have an upper case on "Exemplaire" in the checkin and checkout note?
   Actually I would simplify, remove this text and the item barcode, and display only the note content.

I would let the barcode. If we check-in several items and don't see immediately the note, it's easier to find back the good item with it, no ?

When circulation operations are done, if item contains the corresponding
circulation note, this note will be displayed as a permanent toastr
message.

* refactoring item circulation collapsed informations design.
* adds postprocessRecord behavior to avoid empty notes array on item.

Co-authored-by : Renaud Michotte <renaud.michotte@gmail.com>
@zannkukai zannkukai force-pushed the zan-#item-notes-circulation-messages branch from 8ed0dcd to 557ac5d Compare June 16, 2020 07:35
@zannkukai
Copy link
Contributor Author

@JoelleDosimont It seems we found a good solution with a "callout" wrapper adding a warning border around the item with a note + sticky icon.
https://files.gitter.im/zannkukai/95sC/image.png

@zannkukai zannkukai merged commit 4a31735 into rero:dev Jun 20, 2020
@zannkukai zannkukai deleted the zan-#item-notes-circulation-messages branch June 20, 2020 07:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants