Skip to content

fix(DataIterable): do not update selection if totalItems is set - #7396

Merged
johnleider merged 1 commit into
vuetifyjs:stablefrom
florentchauveau:fix/issue-6094
Aug 14, 2019
Merged

fix(DataIterable): do not update selection if totalItems is set#7396
johnleider merged 1 commit into
vuetifyjs:stablefrom
florentchauveau:fix/issue-6094

Conversation

@florentchauveau

@florentchauveau florentchauveau commented Jun 4, 2019

Copy link
Copy Markdown

Description

Selection should not be changed when totalItems is set (that is when server-side pagination is involved).

Motivation and Context

Fixes #6472

How Has This Been Tested?

unit

Markup:

Details
<template>
  <v-data-table
    v-model="selected"
    select-all
    :headers="headers"
    :items="desserts"
    item-key="name"
    :pagination.sync="pagination"
    :total-items="totalDesserts"
    :loading="loading"
    class="elevation-1"
  >
    <template slot="items" slot-scope="props">
      <td>
        <v-checkbox
          v-model="props.selected"
          primary
          hide-details
        ></v-checkbox>
      </td>
      <td>{{ props.item.name }}</td>
      <td class="text-xs-right">{{ props.item.calories }}</td>
      <td class="text-xs-right">{{ props.item.fat }}</td>
      <td class="text-xs-right">{{ props.item.carbs }}</td>
      <td class="text-xs-right">{{ props.item.protein }}</td>
      <td class="text-xs-right">{{ props.item.iron }}</td>
    </template>
  </v-data-table>
</template>

<script>
export default {
  data () {
    return {
      selected: [],
      totalDesserts: 0,
      desserts: [],
      loading: true,
      pagination: {},
      headers: [
        {
          text: 'Dessert (100g serving)',
          align: 'left',
          sortable: false,
          value: 'name'
        },
        { text: 'Calories', value: 'calories' },
        { text: 'Fat (g)', value: 'fat' },
        { text: 'Carbs (g)', value: 'carbs' },
        { text: 'Protein (g)', value: 'protein' },
        { text: 'Iron (%)', value: 'iron' }
      ]
    }
  },
  watch: {
    pagination: {
      handler () {
        this.getDataFromApi()
          .then(data => {
            this.desserts = data.items
            this.totalDesserts = data.total
          })
      },
      deep: true
    }
  },
  mounted () {
    this.getDataFromApi()
      .then(data => {
        this.desserts = data.items
        this.totalDesserts = data.total
      })
  },
  methods: {
    getDataFromApi () {
      this.loading = true
      return new Promise((resolve, reject) => {
        const { sortBy, descending, page, rowsPerPage } = this.pagination

        let items = this.getDesserts()
        const total = items.length

        if (this.pagination.sortBy) {
          items = items.sort((a, b) => {
            const sortA = a[sortBy]
            const sortB = b[sortBy]

            if (descending) {
              if (sortA < sortB) return 1
              if (sortA > sortB) return -1
              return 0
            } else {
              if (sortA < sortB) return -1
              if (sortA > sortB) return 1
              return 0
            }
          })
        }

        if (rowsPerPage > 0) {
          items = items.slice((page - 1) * rowsPerPage, page * rowsPerPage)
        }

        setTimeout(() => {
          this.loading = false
          resolve({
            items,
            total
          })
        }, 1000)
      })
    },
    getDesserts () {
      return [
        {
          name: 'Frozen Yogurt',
          calories: 159,
          fat: 6.0,
          carbs: 24,
          protein: 4.0,
          iron: '1%'
        },
        {
          name: 'Ice cream sandwich',
          calories: 237,
          fat: 9.0,
          carbs: 37,
          protein: 4.3,
          iron: '1%'
        },
        {
          name: 'Eclair',
          calories: 262,
          fat: 16.0,
          carbs: 23,
          protein: 6.0,
          iron: '7%'
        },
        {
          name: 'Cupcake',
          calories: 305,
          fat: 3.7,
          carbs: 67,
          protein: 4.3,
          iron: '8%'
        },
        {
          name: 'Gingerbread',
          calories: 356,
          fat: 16.0,
          carbs: 49,
          protein: 3.9,
          iron: '16%'
        },
        {
          name: 'Jelly bean',
          calories: 375,
          fat: 0.0,
          carbs: 94,
          protein: 0.0,
          iron: '0%'
        },
        {
          name: 'Lollipop',
          calories: 392,
          fat: 0.2,
          carbs: 98,
          protein: 0,
          iron: '2%'
        },
        {
          name: 'Honeycomb',
          calories: 408,
          fat: 3.2,
          carbs: 87,
          protein: 6.5,
          iron: '45%'
        },
        {
          name: 'Donut',
          calories: 452,
          fat: 25.0,
          carbs: 51,
          protein: 4.9,
          iron: '22%'
        },
        {
          name: 'KitKat',
          calories: 518,
          fat: 26.0,
          carbs: 65,
          protein: 7,
          iron: '6%'
        }, 
        {
          name: 'Frozen KitKat',
          calories: 159,
          fat: 6.0,
          carbs: 24,
          protein: 4.0,
          iron: '1%'
        },
        {
          name: 'Ice cream Donut',
          calories: 237,
          fat: 9.0,
          carbs: 37,
          protein: 4.3,
          iron: '1%'
        },
        {
          name: 'Eclair sandwich',
          calories: 262,
          fat: 16.0,
          carbs: 23,
          protein: 6.0,
          iron: '7%'
        },
        {
          name: 'Cupcake Yogurt',
          calories: 305,
          fat: 3.7,
          carbs: 67,
          protein: 4.3,
          iron: '8%'
        },
        {
          name: 'Gingerbread house',
          calories: 356,
          fat: 16.0,
          carbs: 49,
          protein: 3.9,
          iron: '16%'
        },
      ]
    }
  }
}
</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 feature but make 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 breaking 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)

@codecov

codecov Bot commented Jun 4, 2019

Copy link
Copy Markdown

Codecov Report

Merging #7396 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #7396      +/-   ##
==========================================
+ Coverage   85.43%   85.43%   +<.01%     
==========================================
  Files         298      298              
  Lines        7221     7222       +1     
  Branches     1804     1805       +1     
==========================================
+ Hits         6169     6170       +1     
  Misses        956      956              
  Partials       96       96
Impacted Files Coverage Δ
packages/vuetify/src/mixins/data-iterable.js 80.37% <100%> (+0.12%) ⬆️

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 b57a7be...44bc1da. Read the comment docs.

2 similar comments
@codecov

codecov Bot commented Jun 4, 2019

Copy link
Copy Markdown

Codecov Report

Merging #7396 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #7396      +/-   ##
==========================================
+ Coverage   85.43%   85.43%   +<.01%     
==========================================
  Files         298      298              
  Lines        7221     7222       +1     
  Branches     1804     1805       +1     
==========================================
+ Hits         6169     6170       +1     
  Misses        956      956              
  Partials       96       96
Impacted Files Coverage Δ
packages/vuetify/src/mixins/data-iterable.js 80.37% <100%> (+0.12%) ⬆️

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 b57a7be...44bc1da. Read the comment docs.

@codecov

codecov Bot commented Jun 4, 2019

Copy link
Copy Markdown

Codecov Report

Merging #7396 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #7396      +/-   ##
==========================================
+ Coverage   85.43%   85.43%   +<.01%     
==========================================
  Files         298      298              
  Lines        7221     7222       +1     
  Branches     1804     1805       +1     
==========================================
+ Hits         6169     6170       +1     
  Misses        956      956              
  Partials       96       96
Impacted Files Coverage Δ
packages/vuetify/src/mixins/data-iterable.js 80.37% <100%> (+0.12%) ⬆️

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 b57a7be...44bc1da. Read the comment docs.

@nekosaur

nekosaur commented Jun 4, 2019

Copy link
Copy Markdown
Member

Please add a playground markup for illustrating the issue as well.

This is a candidate for a future 1.5 LTS patch. It has already been fixed in 2.0

dsseng
dsseng previously requested changes Jun 5, 2019

@dsseng dsseng left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

  • rebase to next

@dsseng
dsseng dismissed their stale review June 5, 2019 11:39

ah sorry, that's LTS

@florentchauveau

Copy link
Copy Markdown
Author

Please add a playground markup for illustrating the issue as well.

@nekosaur done!

@06b

06b commented Jun 6, 2019

Copy link
Copy Markdown
Contributor

Confirming that it appears to fix #6472

@johnleider johnleider added the S: on hold The issue is on hold until further notice label Jun 9, 2019
@jacekkarczmarczyk jacekkarczmarczyk added T: bug Functionality that does not work as intended/expected S: has merge conflicts The pending Pull Request has merge conflicts and removed S: on hold The issue is on hold until further notice labels Jul 28, 2019
@johnleider johnleider added this to the v2.0.x milestone Aug 1, 2019
@johnleider
johnleider requested a review from nekosaur August 1, 2019 17:57
@johnleider

Copy link
Copy Markdown
Member

We have removed the on hold label. Can you resolve the merge conflicts? Thank you

@nekosaur

nekosaur commented Aug 2, 2019

Copy link
Copy Markdown
Member

This needs to be rebased to stable branch

@florentchauveau

Copy link
Copy Markdown
Author

OK, will work on that today or tomorrow.

@nekosaur nekosaur modified the milestones: v2.0.x, v1.5-lts Aug 3, 2019
@johnleider

Copy link
Copy Markdown
Member

Just a reminder that this will be closed in 4 days if the requested changes are not made.

If you have any additional questions, please reach out to us in our Discord community.

@florentchauveau
florentchauveau changed the base branch from master to stable August 12, 2019 20:52
@florentchauveau

Copy link
Copy Markdown
Author

Sorry for the delay. I have rebased on stable branch.

@jacekkarczmarczyk jacekkarczmarczyk removed the S: has merge conflicts The pending Pull Request has merge conflicts label Aug 13, 2019
@johnleider
johnleider merged commit a2fccb4 into vuetifyjs:stable Aug 14, 2019
@lock lock Bot locked as resolved and limited conversation to collaborators Sep 13, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

C: VDataIterator T: bug Functionality that does not work as intended/expected

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants