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.

Sign up for GitHub

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

Jump to bottom

Circulation category: update display #556

Merged
merged 1 commit into from
Apr 17, 2021
Merged

Circulation category: update display #556

merged 1 commit into from
Apr 17, 2021

Conversation

zannkukai
Copy link
Contributor

@zannkukai zannkukai commented Mar 22, 2021 •
edited
Loading

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

Code review check list

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

@zannkukai zannkukai self-assigned this Mar 22, 2021
@zannkukai zannkukai force-pushed the zan-itty-display branch 3 times, most recently from ba95f80 to ec8ec41 Compare March 29, 2021 11:26
@zannkukai zannkukai marked this pull request as ready for review March 29, 2021 11:26
@zannkukai zannkukai added this to the v1.2.0 milestone Mar 29, 2021
@zannkukai zannkukai force-pushed the zan-itty-display branch 2 times, most recently from e847569 to e1f57d6 Compare March 29, 2021 12:38
Garfield-fr
Garfield-fr approved these changes Mar 30, 2021
View reviewed changes
Copy link
Contributor

@Garfield-fr Garfield-fr left a comment

Choose a reason for hiding this comment

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

If you add functions in the ts file for the template, it would be nice to put changeDetection: ChangeDetectionStrategy.OnPush on the @component parameter

.../admin/src/app/record/detail-view/item-type-detail-view/item-type-detail-view.component.html Outdated Show resolved Hide resolved
@zannkukai zannkukai force-pushed the zan-itty-display branch 2 times, most recently from ea74530 to 4ba0756 Compare March 30, 2021 13:48
@pronguen pronguen self-requested a review March 30, 2021 14:38
pronguen
pronguen reviewed Mar 30, 2021
View reviewed changes
Copy link
Contributor

@pronguen pronguen left a comment •
edited by zannkukai
Loading

Choose a reason for hiding this comment

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

image

  • Circulation information (in yellow in image above)
    • If the interface is in French, but there is no circulation info defined for this language, it should display the circulation info in another language (the first defined). Currently, it displays the item type name in this case.
    • If there is no circulation info defined in any languages, it should not display anything. The item type name is never displayed as it is an information for the professional.
    • [not blocking] It should not be possible, in the item type, to define two different circulation info for the same language. Currently, the system blocks only if there are two for the same language with the exact same text.
      image
  • Displayed status (negative availability)
    • [not blocking] I would make this field required (at least have one language). Currently, if no displayed status is defined, the item type name is displayed (not wanted).
    • If the interface is in French, but there is no displayed status defined for this language, it should display the status in another language (the first defined). Currently, it displays the item type name in this case.
    • [not blocking] It should not be possible, in the item type, to define two different displayed status for the same language. Currently, the system blocks only if there are two for the same language with the exact same text.
  • Availability
    • You implemented the availability display in the professional interface 👍 👍 👍 This is great, because it was really annoying!
    • [not blocking] The document in image below has two items, both of them have a temporary item type "missing" (not available). The one of the pro view is marked as available, the other as not available.
      image

@zannkukai zannkukai force-pushed the zan-itty-display branch 4 times, most recently from 44d0e11 to 36d995e Compare March 31, 2021 13:59
jma
jma approved these changes Apr 13, 2021
View reviewed changes
...dmin/src/app/record/detail-view/circ-policy-detail-view/circ-policy-detail-view.component.ts Show resolved Hide resolved
...l-view/document-detail-view/holding/default-holding-item/default-holding-item.component.html Show resolved Hide resolved
...l-view/document-detail-view/holding/default-holding-item/default-holding-item.component.html Outdated Show resolved Hide resolved
...l-view/document-detail-view/holding/default-holding-item/default-holding-item.component.html Outdated Show resolved Hide resolved
...l-view/document-detail-view/holding/default-holding-item/default-holding-item.component.html Outdated Show resolved Hide resolved
.../admin/src/app/record/detail-view/item-type-detail-view/item-type-detail-view.component.html Show resolved Hide resolved
projects/admin/src/app/scss/variables.scss Show resolved Hide resolved
projects/public-search/src/app/api/item-api.service.ts Show resolved Hide resolved
projects/public-search/src/app/document-detail/item/item.component.html Outdated Show resolved Hide resolved
@zannkukai
Circulation category: update display
2b3ecfc 
* Updates display of circulation category detailed view with new fields.
* Use the circulation informations labels instead of category name if
  the label is available in the current used UI language.
* Updates the "availability" colored circle and text behavior according
  to new fields.
* Closes rero/rero-ils#1586.

Co-authored-by: Renaud Michotte <renaud.michotte@gmail.com>
@zannkukai zannkukai merged commit b02bb0b into rero:dev Apr 17, 2021
@zannkukai zannkukai deleted the zan-itty-display branch April 26, 2021 15:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pronguen pronguen pronguen left review comments

@Garfield-fr Garfield-fr Garfield-fr approved these changes

@jma jma jma approved these changes

@sebdeleze sebdeleze sebdeleze approved these changes

Assignees

@zannkukai zannkukai

Labels
None yet
Projects
None yet
Milestone
v1.2.0
Development

Successfully merging this pull request may close these issues.

The availability of a holdings of type "serial" with no items should not be "no items received"
5 participants
@zannkukai @Garfield-fr @jma @sebdeleze @pronguen