patron profile: add sort by due date #774

vgranata · 2021-12-20T18:56:03Z

Add sort for due date in patron profile view.

Co-Authored-by: Valeria Granata valeria@chaw.com

Why are you opening this PR?

Code review check list

Garfield-fr · 2022-02-03T11:32:08Z

.../public-search/src/app/patron-profile/patron-profile-loans/patron-profile-loans.component.ts

+  /** Initial records load and sort */
+  private _initialLoadSorted(): void {
+    const pgr = this._paginator;
+    this._loanQueryAll().subscribe((response: Record) => {


It is not a good idea to load all the loans at once, because if there are a lot of them, it will make the thing explode. The pager is there to avoid this.

You really need to use the pager.

+1. You should update the "_loanApiService.getOnLoan()" method to accept a sorting option. This options could be use into

rero-ils-ui/projects/public-search/src/app/api/loan-api.service.ts

Lines 54 to 56 in f8e0abb

return this._recordService.getRecords(

'loans', this._patronStateQuery(patronPid, loanStates), page, itemsPerPage,

undefined, undefined, headers

to sort result.
Any time a user change the sort criteria clicking on column header, the records must be reloaded on page 1 (even if user is on page 3 at click time)

...ublic-search/src/app/patron-profile/patron-profile-loans/patron-profile-loans.component.html

zannkukai · 2022-02-06T14:15:31Z

I continue about thinking at this problem during this weekend.
My personal conclusion is that paginator is maybe not a good idea on this page. As an user, I could be great to see all my current loans with one page without click on paginator links.

About technical it should be test with an account with a lot of loans (~100 loans) to check performance.
It should be discuss with other devs/PO members during a daily.

zannkukai · 2022-02-09T08:36:32Z

.../public-search/src/app/patron-profile/patron-profile-loans/patron-profile-loans.component.ts

@@ -29,14 +29,17 @@ import { PatronProfileMenuService } from '../patron-profile-menu.service';
 export class PatronProfileLoansComponent implements OnInit, OnDestroy {

  /** Observable subscription */
-  private _subscription = new Subscription();
+  _subscription: Subscription;


Why this change ? Especially if you create an subscription into ngOnInit

zannkukai · 2022-02-09T08:40:41Z

.../public-search/src/app/patron-profile/patron-profile-loans/patron-profile-loans.component.ts

+        this.ngOnDestroy();
+        this.ngOnInit();


Arggh. Is it really a good idea to call life cycle hook function like that ? What's the feeling of @Garfield-fr and/or @jma.

zannkukai · 2022-02-09T08:43:29Z

projects/public-search/src/app/scss/styles.scss

+  color: white;
+}
+select#sort-criteria.custom-select {
+  background-color:#343a40;


is it possible to use bootstrap variable here ? maybe $table-dark-color ?

The changes in this file are to overwrite the style of the sort-list ng-core widget.
I'm not sure how to use bootstrap in this case.

@vgranata

You can check https://github.com/rero/rero-ils-ui/blob/staging/projects/admin/src/app/acquisition/acquisition.scss (and some other files)

@import '~bootstrap/scss/_functions'; @import '~bootstrap/scss/_variables'; @import '/projects/admin/src/app/scss/variables';

allows to use bootstrap variables, bootstrap functions and project variables.

margin-bottom: map-get($spacers, 1);

you can use 1 (small gap space) to 5 (big gap space)
If you can't use a bootstrap variable for the background color ; define a project variable into variables.scss and use this variable in this file

zannkukai · 2022-02-09T08:43:42Z

projects/public-search/src/app/scss/styles.scss

+  background-color:#343a40;
+}
+.input-group-text {
+  background-color:#343a40;


zannkukai · 2022-02-09T08:44:28Z

projects/public-search/src/app/scss/styles.scss

+.input-group-text {
+  background-color:#343a40;
+  border: none;
+  padding: 1px;


use bootstrap function map-get if possible :: map-get($spacers, 1)

zannkukai

still some changes into SCSS file ;-)

zannkukai · 2022-03-17T08:58:58Z

projects/public-search/src/app/scss/styles.scss

+  color: white;
+}
+select#sort-criteria.custom-select {
+  background-color:#343a40;


@vgranata

You can check https://github.com/rero/rero-ils-ui/blob/staging/projects/admin/src/app/acquisition/acquisition.scss (and some other files)

@import '~bootstrap/scss/_functions'; @import '~bootstrap/scss/_variables'; @import '/projects/admin/src/app/scss/variables';

allows to use bootstrap variables, bootstrap functions and project variables.

margin-bottom: map-get($spacers, 1);

you can use 1 (small gap space) to 5 (big gap space)
If you can't use a bootstrap variable for the background color ; define a project variable into variables.scss and use this variable in this file

Garfield-fr · 2022-03-17T11:16:19Z

.../public-search/src/app/patron-profile/patron-profile-loans/patron-profile-loans.component.ts

-  /** OnDestroy hook */
-  ngOnDestroy(): void {
-    this._subscription.unsubscribe();
  }


Do not delete this code

Garfield-fr · 2022-03-17T11:17:49Z

.../public-search/src/app/patron-profile/patron-profile-loans/patron-profile-loans.component.ts

+  selectingSortCriteria(sortCriteria: string) {
+    this.sortCriteria = sortCriteria;
+    this._subscription.unsubscribe();
+    this._initialisePaginatorAndSubscription();


Use le current paginator and change only le value. Not necessary to reinitialize the paginator.

Garfield-fr · 2022-03-17T11:19:20Z

.../public-search/src/app/patron-profile/patron-profile-loans/patron-profile-loans.component.ts

+   */
+  selectingSortCriteria(sortCriteria: string) {
+    this.sortCriteria = sortCriteria;
+    this._subscription.unsubscribe();


Remove this code and change with comment below

PascalRepond · 2022-03-29T11:32:17Z

Works as intended.

Non-blocking: The UI of the sort column dropdown could be better. I would keep a similar look as the one in the pro UI. Keep the table heading color, add a fine border, and add the double arrow to the right :

NPR: problem here is that we barely see that the option is clickable: low contrast of the button + no cursor mousehover
NPR: The sort option disappears on mobile phones (and this view should be mobile first...)
NPR: Could we change the label of the sort to have: "Due date (earliest)" and "Due date (latest)"?
- I think the end user (children, old people, etc.) don't understand easily "date ascending"
- This would have the same scheme than for document for ex ("Date (oldest)", "Date (newest)")
PRE: Sort menu should only be visible when "current loans" tab is active, for now. When other tabs are active, it becomes invisible.

Co-Authored-by: Valeria Granata <valeria@chaw.com>

github-actions bot added f: circulation Concerns the circulation interface or backend f: public ui Public interface, as opposed to the professional interface f: user management labels Dec 20, 2021

vgranata force-pushed the grv-2263-loans-sorted-by-due-date branch from 971b6ba to dbbfa2d Compare December 20, 2021 19:02

vgranata force-pushed the grv-2263-loans-sorted-by-due-date branch from dbbfa2d to f8e0abb Compare February 3, 2022 10:28

vgranata marked this pull request as ready for review February 3, 2022 10:38

vgranata requested review from Garfield-fr and zannkukai February 3, 2022 10:41

Garfield-fr requested changes Feb 3, 2022

View reviewed changes

zannkukai reviewed Feb 4, 2022

View reviewed changes

...ublic-search/src/app/patron-profile/patron-profile-loans/patron-profile-loans.component.html Outdated Show resolved Hide resolved

vgranata force-pushed the grv-2263-loans-sorted-by-due-date branch 2 times, most recently from a955266 to b5d9e96 Compare February 8, 2022 16:14

vgranata changed the title ~~patron profile: add sort for title, library and due date~~ patron profile: add sort for due date Feb 8, 2022

vgranata changed the title ~~patron profile: add sort for due date~~ patron profile: add sort by due date Feb 8, 2022

vgranata requested review from Garfield-fr and zannkukai February 9, 2022 08:04

zannkukai reviewed Feb 9, 2022

View reviewed changes

vgranata force-pushed the grv-2263-loans-sorted-by-due-date branch 2 times, most recently from 3bbe84a to 1a95a13 Compare March 16, 2022 13:19

vgranata requested a review from zannkukai March 16, 2022 15:08

zannkukai reviewed Mar 17, 2022

View reviewed changes

PascalRepond requested a review from jma March 17, 2022 11:14

Garfield-fr requested changes Mar 17, 2022

View reviewed changes

jma removed their request for review March 17, 2022 12:25

vgranata force-pushed the grv-2263-loans-sorted-by-due-date branch from 1a95a13 to 2eab456 Compare March 24, 2022 07:52

github-actions bot added the f: professional ui Professional interface label Mar 24, 2022

vgranata force-pushed the grv-2263-loans-sorted-by-due-date branch 2 times, most recently from 7085bb5 to 65c98d0 Compare March 24, 2022 08:18

vgranata requested review from zannkukai and Garfield-fr March 24, 2022 08:47

vgranata force-pushed the grv-2263-loans-sorted-by-due-date branch 4 times, most recently from e3c2c9f to caa954f Compare March 24, 2022 11:02

Garfield-fr approved these changes Mar 24, 2022

View reviewed changes

vgranata force-pushed the grv-2263-loans-sorted-by-due-date branch from caa954f to 6d4587f Compare March 24, 2022 12:39

vgranata requested a review from PascalRepond March 24, 2022 13:30

zannkukai approved these changes Mar 24, 2022

View reviewed changes

vgranata force-pushed the grv-2263-loans-sorted-by-due-date branch from 6d4587f to a4ceabb Compare March 30, 2022 12:46

github-actions bot removed the f: professional ui Professional interface label Mar 30, 2022

vgranata force-pushed the grv-2263-loans-sorted-by-due-date branch from a4ceabb to a5bff64 Compare March 30, 2022 12:51

vgranata requested review from Garfield-fr and zannkukai March 30, 2022 13:28

PascalRepond approved these changes Mar 31, 2022

View reviewed changes

vgranata force-pushed the grv-2263-loans-sorted-by-due-date branch from a5bff64 to f09b5f4 Compare March 31, 2022 13:50

patron profile: add sort of due date in loans

de05524

Co-Authored-by: Valeria Granata <valeria@chaw.com>

vgranata force-pushed the grv-2263-loans-sorted-by-due-date branch from f09b5f4 to de05524 Compare March 31, 2022 14:54

PascalRepond approved these changes Apr 1, 2022

View reviewed changes

PascalRepond merged commit a932e12 into rero:staging Apr 1, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

patron profile: add sort by due date #774

patron profile: add sort by due date #774

vgranata commented Dec 20, 2021 •

edited by PascalRepond

Loading

Garfield-fr Feb 3, 2022

zannkukai Feb 4, 2022

zannkukai commented Feb 6, 2022

zannkukai Feb 9, 2022 •

edited

Loading

zannkukai Feb 9, 2022

zannkukai Feb 9, 2022

vgranata Mar 16, 2022

zannkukai Mar 17, 2022

zannkukai Feb 9, 2022

zannkukai Feb 9, 2022

zannkukai left a comment

zannkukai Mar 17, 2022

Garfield-fr Mar 17, 2022

Garfield-fr Mar 17, 2022

Garfield-fr Mar 17, 2022

PascalRepond commented Mar 29, 2022 •

edited

Loading

	return this._recordService.getRecords(
	'loans', this._patronStateQuery(patronPid, loanStates), page, itemsPerPage,
	undefined, undefined, headers

patron profile: add sort by due date #774

patron profile: add sort by due date #774

Conversation

vgranata commented Dec 20, 2021 • edited by PascalRepond Loading

Why are you opening this PR?

Code review check list

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zannkukai commented Feb 6, 2022

zannkukai Feb 9, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zannkukai left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

PascalRepond commented Mar 29, 2022 • edited Loading

vgranata commented Dec 20, 2021 •

edited by PascalRepond

Loading

zannkukai Feb 9, 2022 •

edited

Loading

PascalRepond commented Mar 29, 2022 •

edited

Loading