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

Table: make toggleAllSelection method an instance method(#14075) #14146

Merged
merged 1 commit into from Feb 20, 2019
Merged

Table: make toggleAllSelection method an instance method(#14075) #14146

merged 1 commit into from Feb 20, 2019

Conversation

ghost
Copy link

@ghost ghost commented Jan 18, 2019

fix #14075 .
All instances of TableStore share the same toggleAllSelection method in TableStore.prototype.mutations.Since the toggleAllSelection method is a debounce function which create a closure, all TableStore's instances share the same Lexical scope while they call the toggleAllSelection method. The time interval between two tables call toggleAllSelection is less than the debouncing delay(10), so the first call doesn't work.

This commit make the actual toggleAllSelection method an instance method.

Please make sure these boxes are checked before submitting your PR, thank you!

  • Make sure you follow Element's contributing guide (中文 | English | Español).
  • Make sure you are merging your commits to dev branch.
  • Add some descriptions and refer relative issues for you PR.

@element-bot
Copy link
Member

Deploy preview for element ready!

Built with commit c927012

https://deploy-preview-14146--element.netlify.com

@island205 island205 added this to the 2.6.0 milestone Feb 15, 2019
@@ -112,6 +112,35 @@ const TableStore = function(table, initialState = {}) {
selectOnIndeterminate: false
};

this._toggleAllSelection = debounce(10, function(states) {
Copy link
Contributor

Choose a reason for hiding this comment

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

this.toggleAllSelection=debunce(10, this._toggleAllSelection)

Copy link
Author

Choose a reason for hiding this comment

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

这里是为了避免每次实例化时都创建一个匿名函数吗?这里可以把传入debounce的函数声明成一个常量,这样在原型链也查找不到了。

table.$emit('select-all', selection);
states.isAllSelected = value;
})
toggleAllSelection(state) {
Copy link
Contributor

Choose a reason for hiding this comment

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

_toggleAllSelection(state) {

Copy link
Author

Choose a reason for hiding this comment

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

执行toggleAllSelection方法时commit的操作是toggleAllSelection,所以TableStore.prototype.method要有一个同名方法toggleAllSelection。

@island205 island205 self-assigned this Feb 15, 2019
@island205
Copy link
Contributor

@LilyWakana please move _toggleAllSelection to the prototype method

@island205 island205 merged commit 7b1d0e2 into ElemeFE:dev Feb 20, 2019
@island205
Copy link
Contributor

Great work, Thank you for your contributions!

@island205
Copy link
Contributor

by the way:

let o = {
  x() {
    console.log('x')
  }
}
let x = function () {
  console.log('y')
  this.x()
}
x.call(o)

The best solution is: put toggleAllSelection to the TableStore.prototype, add this.toggleAllSelection=debunce(10, this.toggleAllSelection) to the TableStore constructor funciton.

@ghost
Copy link
Author

ghost commented Feb 20, 2019

by the way:

let o = {
  x() {
    console.log('x')
  }
}
let x = function () {
  console.log('y')
  this.x()
}
x.call(o)

The best solution is: put toggleAllSelection to the TableStore.prototype, add this.toggleAllSelection=debunce(10, this.toggleAllSelection) to the TableStore constructor funciton.

got it.thank you.

zhengsixsix added a commit to zhengsixsix/element-plus that referenced this pull request May 22, 2023
ElemeFE/element#14146 this problem led to the selection without the first to
get the value

closed element-plus#12887
zhengsixsix added a commit to zhengsixsix/element-plus that referenced this pull request May 22, 2023
ElemeFE/element#14146 this problem led to the selection without the first to
get the value

closed element-plus#12887
zhengsixsix added a commit to zhengsixsix/element-plus that referenced this pull request May 23, 2023
ElemeFE/element#14146 this problem led to the selection without the first to
get the value

closed element-plus#12887
zhengsixsix added a commit to zhengsixsix/element-plus that referenced this pull request May 24, 2023
ElemeFE/element#14146 this problem led to the selection without the first to
get the value

closed element-plus#12887
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.

[Bug Report] el-table method toggleAllSelection on mounted event works only for the last call
2 participants