Skip to content

fix(table): fix filtered event, fix emptyFilter message w/filter function, fix reactivity of filter sub routines (Fixes #1989,#1517) #2149

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

Merged
merged 67 commits into from
Nov 11, 2018

Conversation

tmorehouse
Copy link
Member

@tmorehouse tmorehouse commented Nov 7, 2018

Description of PR:

Adds additional checks to see if the filtered rows have actually changed.

Also prevents the filtered event from firing on the initial render of the table.

Previously, b-table was only looking at changes in length of the filtered rows. The new added checks (looseEqual check and filter prop check) help ensure proper reactivity and proper displaying of the empty-filtered-text with user supplied filter functions

Fixes #1989
Fixes #1517

Undoes #1893 which broke efficient reactivity for filtering, sorting and paging (it was using non-reactive methods when computedProps should have been used for efficient caching)

Fixes ARIA issue introduced by PR #1587


PR checklist:

What kind of change does this PR introduce? (check at least one)

  • Bugfix
  • Feature
  • Enhancement to an existing feature
  • ARIA accessibility
  • Documentation update
  • Other, please describe:

Does this PR introduce a breaking change? (check one)

  • Yes
  • No

For filtering using an external filter function, users should use the filter-function instead of setting filter to a function.

The external filter function (if provided) will also receive a copy of the filter prop as the second argument (the first argument remains the same, which is the record being inspected).

it is still possible to set filtered to a filter function (as long as filter-function is left null), but it is discouraged (deprecated), as it was not overly reactive to changes (i.e. requiring a forced refresh of the table to apply the changes, or passing a new function ref to filter.

The new way allows the user to pass a string, object or array to filter to perform more complex filtering, and allows the filter function to be reactive to changes in the filter prop.

The PR fulfills these requirements:

  • It's submitted to the dev branch, not the master branch
  • When resolving a specific issue, it's referenced in the PR's title (e.g. fixes #xxxx[,#xxxx], where "xxxx" is the issue number)

If new features/enhancement/fixes are added or changed:

  • Includes documentation updates
  • New/updated tests are included and passing (if required)
  • Existing test suites are passing
  • The changes have not impacted the functionality of other components or directives
  • ARIA Accessibility has been taken into consideration (does it affect screen reader users or keyboard only users?)

If adding a new feature, or changing the functionality of an existing feature, the PR's description includes:

  • A convincing reason for adding this feature (to avoid wasting your time, it's best to open a suggestion issue first and wait for approval before working on it)

PR titles should following the Conventional Commits naming convention

@codecov
Copy link

codecov bot commented Nov 7, 2018

Codecov Report

Merging #2149 into dev will increase coverage by 0.35%.
The diff coverage is 83.01%.

Impacted file tree graph

@@            Coverage Diff            @@
##              dev   #2149      +/-   ##
=========================================
+ Coverage   65.04%   65.4%   +0.35%     
=========================================
  Files         159     159              
  Lines        2958    3000      +42     
  Branches      812     834      +22     
=========================================
+ Hits         1924    1962      +38     
- Misses        748     751       +3     
- Partials      286     287       +1
Impacted Files Coverage Δ
src/components/table/table.js 75.84% <83.01%> (+1.5%) ⬆️
src/utils/loose-equal.js 85.71% <0%> (+4.76%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5e5c5c9...97b9ade. Read the comment docs.

filtered event check moved to the filteredItems() method.

Filtered event is now emitted after the table has updated (via $nextTick).

Improves run-time memory footprint by not maintaining a separate copy of the filtered data.
Handles case where filter is a function and `empty` message instead of `emptyFiltered` message was showing when filtered items was 0 length array.
@tmorehouse tmorehouse changed the title fix(table): Only emit filtered event if filtered rows has changed. Fixes #1989 fix(table): filtered event fixes and fix emptyFilter message with filter function (Fixes #1989,#1517) Nov 7, 2018
@tmorehouse tmorehouse changed the title fix(table): filtered event fixes and fix emptyFilter message with filter function (Fixes #1989,#1517) [WIP] fix(table): filtered event fixes and fix emptyFilter message with filter function (Fixes #1989,#1517) Nov 7, 2018
@tmorehouse
Copy link
Member Author

Just needs a few documentation tweaks before merge

@tmorehouse
Copy link
Member Author

filtering, sorting and pagination are now fully reactive via computed props, fixing what was broken via #1893 (using methods when computedProps should have been used)

To prevent undefined values from creeping in
When the fields's label was set to a blank string, this left non-sighted users at a disadvantage as to what the column is about.

This adds the field's key (humanized) to the `aria-label` to provides hint to non-sighted users.

Alternatively if a field title is provided, then the aria-label is not altered.

Addresses ARIA issues introduced by PR #1587
@tmorehouse tmorehouse requested review from mosinve and pi0 November 10, 2018 21:12
@tmorehouse
Copy link
Member Author

Additional separate PRs will be created for adding:

  • provider function update debouncing
  • filtering and sorting based on formatted values and scopedSlots (converting them to plain text), and only on columns in the fields definition
  • Tri-state column sorting (i.e. asc => desc => none => acs => etc)

@tmorehouse tmorehouse changed the title [WIP] fix(table): filtered event fixes and fix emptyFilter message with filter function (Fixes #1989,#1517) [WIP] fix(table): fix filtered event, fix emptyFilter message w/filter function, fix reactivity of filter sub routines (Fixes #1989,#1517) Nov 11, 2018
@tmorehouse tmorehouse changed the title [WIP] fix(table): fix filtered event, fix emptyFilter message w/filter function, fix reactivity of filter sub routines (Fixes #1989,#1517) fix(table): fix filtered event, fix emptyFilter message w/filter function, fix reactivity of filter sub routines (Fixes #1989,#1517) Nov 11, 2018
@tmorehouse tmorehouse merged commit e0e1eee into dev Nov 11, 2018
@tmorehouse tmorehouse mentioned this pull request Nov 14, 2018
89 tasks
@tmorehouse tmorehouse deleted the tmorehouse/table-filtered branch November 17, 2018 03:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improvement: Fire <b-table> filtered-event when filteredItems array gets modified table: empty filtered text shows unexpectedly when using a function
1 participant