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

fix(VDataTable): include rows that exclusively match custom filters #11181

Merged
merged 2 commits into from
May 4, 2020

Conversation

Ignigena
Copy link
Contributor

@Ignigena Ignigena commented Apr 22, 2020

Description

Currently, if a custom filter is used on a header config, it won't evaluate on any search term that exclusively match this custom filter. The term must match one or more columns that fall under headersWithoutCustomFilters.

Closes #9887

Motivation and Context

Behaviour is inconsistent since the custom filter will be evaluated up until the point that a search term is entered. Once a term is entered, even if the term matches the custom filter, the row is excluded from the results if none of the other columns match.

A simple example is provided in the Codepen in the linked issue: by adding a custom filter with a case-sensitive search for Dessert Name, the ability to search is lost.

How Has This Been Tested?

  • Tested in our own project which uses Vuetify
  • Playground reproduction (below)
  • Added a unit test verifying the Codepen/playground reproduction

Markup:

<template>
  <v-data-table
    :headers="headers"
    :items="desserts"
    :search="search"
    item-key="name"
  >
    <template v-slot:top>
      <v-text-field v-model="search" label="Search (EXACT MATCH)" class="mx-4"></v-text-field>
    </template>
  </v-data-table>
</template>

<script>
  export default {
    data: () => ({
      search: '',
      headers: [
        {
          text: 'Dessert (100g serving)',
          align: 'left',
          sortable: false,
          value: 'name',
          filter: (value, search) => {
            if (!search) return true
            return value === search
          }
        },
        { text: 'Calories', value: 'calories' },
        { text: 'Fat (g)', value: 'fat' },
        { text: 'Carbs (g)', value: 'carbs' },
        { text: 'Protein (g)', value: 'protein' },
        { text: 'Iron (%)', value: 'iron' }
      ],
      desserts: [
        {
          value: false,
          name: 'Frozen Yogurt',
          calories: 159,
          fat: 6.0,
          carbs: 24,
          protein: 4.0,
          iron: '1%',
          brands: [
            {
              name: 'Blue Bell',
              calories: 237,
              fat: 9.0,
              carbs: 37,
              protein: 4.3,
              iron: '1%'
            },
            {
              name: 'Tilamook',
              calories: 262,
              fat: 16.0,
              carbs: 23,
              protein: 6.0,
              iron: '7%'
            }
          ]
        },
        {
          value: false,
          name: 'Ice cream sandwich',
          calories: 237,
          fat: 9.0,
          carbs: 37,
          protein: 4.3,
          iron: '1%'
        },
        {
          value: false,
          name: 'Eclair',
          calories: 262,
          fat: 16.0,
          carbs: 23,
          protein: 6.0,
          iron: '7%'
        },
        {
          value: false,
          name: 'Cupcake',
          calories: 305,
          fat: 3.7,
          carbs: 67,
          protein: 4.3,
          iron: '8%'
        },
        {
          value: false,
          name: 'Gingerbread',
          calories: 356,
          fat: 16.0,
          carbs: 49,
          protein: 3.9,
          iron: '16%'
        },
        {
          value: false,
          name: 'Jelly bean',
          calories: 375,
          fat: 0.0,
          carbs: 94,
          protein: 0.0,
          iron: '0%'
        },
        {
          value: false,
          name: 'Lollipop',
          calories: 392,
          fat: 0.2,
          carbs: 98,
          protein: 0,
          iron: '2%'
        },
        {
          value: false,
          name: 'Honeycomb',
          calories: 408,
          fat: 3.2,
          carbs: 87,
          protein: 6.5,
          iron: '45%'
        },
        {
          value: false,
          name: 'Donut',
          calories: 452,
          fat: 25.0,
          carbs: 51,
          protein: 4.9,
          iron: '22%'
        },
        {
          value: false,
          name: 'KitKat',
          calories: 518,
          fat: 26.0,
          carbs: 65,
          protein: 7,
          iron: '6%'
        }
      ]
    })
  }
</script>

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Improvement/refactoring (non-breaking change that doesn't add any features but makes things better)

Checklist:

  • The PR title is no longer than 64 characters.
  • The PR is submitted to the correct branch (master for bug fixes and documentation updates, dev for new features and backwards compatible changes and next for non-backwards compatible changes).
  • My code follows the code style of this project.
  • I've added relevant changes to the documentation (applies to new features and breaking changes in core library)

@TravisBuddy
Copy link

TravisBuddy commented Apr 22, 2020

Hey @Ignigena,
Your changes look good to me!

View build log

TravisBuddy Request Identifier: 6a9e7350-898b-11ea-aae4-d3259c3e8c73


return filtered
return items.filter(item =>
(headersWithCustomFilters.length && headersWithCustomFilters.every(filterFn(item, search, defaultFilter))) ||
Copy link
Member

Choose a reason for hiding this comment

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

this should be broken up so it's easier to understand, possibly with some comments

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I broke up the callback a bit and added some comments so it's a bit easier to parse the flow of logic. Let me know if this is what you were going for or if you'd like something else 🙂

@jacekkarczmarczyk jacekkarczmarczyk added the T: bug Functionality that does not work as intended/expected label May 2, 2020
@jacekkarczmarczyk jacekkarczmarczyk added this to the v2.2.x milestone May 2, 2020
@johnleider
Copy link
Member

Requesting Final comments from @vuetifyjs/contributors @vuetifyjs/core-team

@nekosaur nekosaur self-requested a review May 4, 2020 15:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C: VDataTable VDatatable T: bug Functionality that does not work as intended/expected
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug Report] DataTable seach using custom filter function inside TableHeader doesn't work properly
6 participants