angular: add filtering, sticky headers, and other improvements to eg-grid

Bug #1831788 reported by Galen Charlton
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Evergreen
Fix Released
Wishlist
Unassigned

Bug Description

This bug is for the following improvements to eg-grid:

    This patch enables users to filter results in Angular eg-grids that
    use PCRUD-based data sources.

    Filtering can be enabled in an eg-grid defintion by adding the following
    attribute to <eg-grid>:

      [filterable]="true"

    If, for some reason, a particular column should not be filterable by the
    user, filtering can be disabled by passing false to the [filterable]
    attribute of an <eg-grid-column> element like this:

      <eg-grid-column [sortable]="true" [filterable]="false" path="barcode"></eg-grid-column>

    When filtering is enabled, a new section of the grid header is displayed that
    includes, for each filterable column:

    * A drop-down menu letting the user specify an operator such as
      "is exactly", "exists" (i.e., is not null), "is greater than", and so
      forth. The drop-down also allows the user to clear a filter for a
      specific column or re-apply it after changing the operator.
    * An input widget for setting the value to filter on. The type of input
      displayed depend on the IDL type of the column. For example, a text field
      will use a normal text <input>; an OU field will use an eg-org-select,
      a link to another IDL class will use a combobox, a timestamp field
      will use an eg-date-select, and so forth.
    * A separate display of the current operator.

    When filtering is enabled, the grid will also display a "Remove Filters" button
    in the action bar.

    Under the hood, the widgets for entering filtering parameters expect
    the data source to have a "filters" key that in turn contains a
    dictionary of PCRUD-style filtering conditions indexed by column name.
    Consequently, a grid data source that wants to use filtering should
    look something like this:

        this.acpSource.getRows = (pager: Pager, sort: any[]) => {
            const orderBy: any = {acp: 'id'};
            if (sort.length) {
                orderBy.acp = sort[0].name + ' ' + sort[0].dir;
            }

            // base query to grab everything
            let base: Object = {};
            base[this.idl.classes['acp'].pkey] = {'!=' : null};
            var query: any = new Array();
            query.push(base);

            // and add any filters
            Object.keys(this.acpSource.filters).forEach(key => {
                Object.keys(this.acpSource.filters[key]).forEach(key2 => {
                    query.push(this.acpSource.filters[key][key2]);
                });
            });
            return this.pcrud.search('acp',
                query, {
                flesh: 1,
                flesh_fields: {acp: ['location']},
                offset: pager.offset,
                limit: pager.limit,
                order_by: orderBy
            });
        };

    This patch also adds two related grid options, sticky headers and the ability
    to reload the data source without losing one's current place in page.

    Sticky headers are enabled by adding the following attribute to the
    <eg-grid> element:

      [stickyHeader]="true"

    When this is enabled, as the user scrolls the grid from top to bottom, the
    header row, including the filter controls, will continue to remain visible
    at the top of the viewport until the user scrolls past the end of the
    grid entirely.

    Reloading grids without losing the current paging settings can now be
    done by a caller (such as code that opens an edit modal) invoking a new
    reloadSansPagerReset() method.

This is part of a project to prepare for converting some acquisitions interfaces to Angular. Work on this bug was sponsored by MassLNC, Georgia Public Library Service, Indiana State Library, CW MARS, and King County Library System.

Galen Charlton (gmc)
Changed in evergreen:
milestone: none → 3.4-beta1
importance: Undecided → Wishlist
Revision history for this message
Galen Charlton (gmc) wrote :

Patch(es) for this are available as part of the following branch:

working/collab/gmcharlt/angular-widget-improvements-2019-06
https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/collab/gmcharlt/angular-widget-improvements-2019-06

Patches for the improvements here depends on the patches for the following bugs in that same branch:

bug 1831780
bug 1831783
bug 1831785

Bugs in that branch that the grid improvements do _not_ depend on are:

bug 1831781
bug 1831784
bug 1831786

However, if at all possible, speedy review of the entire branch would be handy.

tags: added: angular pullrequest
Bill Erickson (berick)
Changed in evergreen:
assignee: nobody → Bill Erickson (berick)
status: New → Confirmed
Revision history for this message
Bill Erickson (berick) wrote :

Sign offs for all commits in the working branch posted here:

https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/berick/angular-widget-improvements-2019-06-signoff

There's a lot of great stuff in here and all of my tests were successful.

It might be nice to allow users to disable+hide the filter row, particularly for smaller screens, to save space when the filters are never used. That can come later, though.

I've opted not to merge just yet, in case others wish to review/test, but I think the code is good to go.

tags: added: signedoff
Changed in evergreen:
assignee: Bill Erickson (berick) → nobody
Revision history for this message
Jane Sandberg (sandbergja) wrote :

Agreed about all the excellent stuff in here.

I think that we should not merge the improvements to date-select yet, because that causes several fm-editors to be completely unusable until https://bugs.launchpad.net/evergreen/+bug/1831907 is merged.

I also think that the eg-help-popover Angular component is close, but not quite ready. My comments are here: https://bugs.launchpad.net/evergreen/+bug/1831781/comments/2

Revision history for this message
Galen Charlton (gmc) wrote : Re: [Bug 1831788] Re: angular: add filtering, sticky headers, and other improvements to eg-grid

> I think that we should not merge the improvements to date-select yet,
> because that causes several fm-editors to be completely unusable until
> https://bugs.launchpad.net/evergreen/+bug/1831907 is merged.

Agreed. I will have a patch ready to test for this one today or tomorrow.

> I also think that the eg-help-popover Angular component is close, but
> not quite ready. My comments are here:
> https://bugs.launchpad.net/evergreen/+bug/1831781/comments/2

I've posted a follow-up patch for the popver.

Revision history for this message
Remington Steed (rjs7) wrote :

I know this is minor, but I'd prefer we rename the function "reloadSansPagerReset()" to something more clear, like "reloadWithoutPagerReset()".

Revision history for this message
Galen Charlton (gmc) wrote :

Grabbing this now for a rebase.

Changed in evergreen:
assignee: nobody → Galen Charlton (gmc)
Revision history for this message
Galen Charlton (gmc) wrote :

The rebase is now available at user/gmcharlt/angular-widget-improvements-2019-07-rebase.

This includes:

- patch for bug 1831907
- the reloadSansPagerReset() => reloadWithoutPagerReset() renaming
- updating the example modal per bug 1823041

Changed in evergreen:
assignee: Galen Charlton (gmc) → nobody
Bill Erickson (berick)
Changed in evergreen:
assignee: nobody → Bill Erickson (berick)
Revision history for this message
Bill Erickson (berick) wrote :

Testing... Ran into a dependency issue where the DateSelectComponent wants to use the formatValue pipe (from FormatService), which is declared in EGCommonModule. However, the new CommonWidgetsModule does not import EgCommonModule (with good reason).

I propose another module, EgCoreModule which packages the services, pipes, etc. from core/* into a module which can then be imported by both EGCommonModule and CommonWidgetsModule. If this sounds reasonable, I can push a patch for that.

Revision history for this message
Galen Charlton (gmc) wrote :

That sounds reasonable to me. Thanks, Bill!

Revision history for this message
Bill Erickson (berick) wrote :

Behold another branch:

https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/berick/lp1831788-grid-filters-and-more

1. Adds EgCoreModule as noted above.
2. Adds a few minor repairs -- lingering dialog api migration and missing i18n attrs.

Galen Charlton (gmc)
Changed in evergreen:
assignee: Bill Erickson (berick) → Galen Charlton (gmc)
Revision history for this message
Galen Charlton (gmc) wrote :

I've reviewed and signed off on Bill's follow-ups and added one of my own for a filter value edge case. Branch is user/gmcharlt/lp1831788-grid-mcgriderson

Changed in evergreen:
assignee: Galen Charlton (gmc) → nobody
Revision history for this message
Jane Sandberg (sandbergja) wrote :

Looks ready for me. Thanks for all your work on this, Galen and Bill.

I jumped on the follow-up bandwagon here: user/sandbergja/lp1831788-griddy-mcgridface

Link here: https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/sandbergja/lp1831788-griddy-mcgridface

This adds my signoff to Galen and Bill's commits, and also includes a small follow up commit that:
* Resolves some small linting errors
* Removes some unused import statements
* Removes one variable definition that is never used

Revision history for this message
Jane Sandberg (sandbergja) wrote :

^ Looks ready *to* me

Galen Charlton (gmc)
Changed in evergreen:
assignee: nobody → Galen Charlton (gmc)
Revision history for this message
Galen Charlton (gmc) wrote :

Follow-up looks good to me; branch pushed to master. Thanks, Bill and Jane!

Changed in evergreen:
status: Confirmed → Fix Committed
assignee: Galen Charlton (gmc) → nobody
Revision history for this message
Galen Charlton (gmc) wrote :

Noting that I've pushed to master another follow-up patch repairing an ng xi18n issue found while reviewing another patch.

Galen Charlton (gmc)
Changed in evergreen:
status: Fix Committed → Fix Released
Revision history for this message
Jeff Davis (jdavis-sitka) wrote :

See also bug 1846042.

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.