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(useAntdTable): 重置后 filter 和 sorter 参数丢失的问题 #2486

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

guaijie
Copy link
Contributor

@guaijie guaijie commented Feb 26, 2024

[中文版模板 / Chinese template]

🤔 This is a ...

  • New feature
  • Bug fix
  • Site / documentation update
  • Demo update
  • TypeScript definition update
  • Bundle size optimization
  • Performance optimization
  • Enhancement feature
  • Internationalization
  • Refactoring
  • Code style optimization
  • Test Case
  • Branch merge
  • Other (about what?)

🔗 Related issue link

💡 Background and solution

📝 Changelog

Language Changelog
🇺🇸 English
🇨🇳 Chinese

☑️ Self Check before Merge

⚠️ Please check all items below before review. ⚠️

  • Doc is updated/provided or not needed
  • Demo is updated/provided or not needed
  • TypeScript definition is updated/provided or not needed
  • Changelog is provided or not needed

@CLAassistant
Copy link

CLAassistant commented Feb 26, 2024

CLA assistant check
All committers have signed the CLA.

@guaijie guaijie changed the title 修复重置后 filter 和 sorter 参数丢失的问题 修复useAntdTable 重置后 filter 和 sorter 参数丢失的问题 Feb 26, 2024
@guaijie guaijie changed the title 修复useAntdTable 重置后 filter 和 sorter 参数丢失的问题 修复 useAntdTable 重置后 filter 和 sorter 参数丢失的问题 Feb 26, 2024
@liuyib liuyib changed the title 修复 useAntdTable 重置后 filter 和 sorter 参数丢失的问题 fix(useAntdTable): 重置后 filter 和 sorter 参数丢失的问题 Feb 26, 2024
@liuyib
Copy link
Collaborator

liuyib commented Feb 26, 2024

@guaijie 试了下没复现出来这个 PR 要解决的问题,麻烦来个在线 demo 看看?

@liuyib liuyib added the 🤔 Need Reproduce We cannot reproduce your problem label Feb 26, 2024
@liuyib

This comment was marked as outdated.

@liuyib
Copy link
Collaborator

liuyib commented Feb 27, 2024

@guaijie 我复现了,不用改 codesandbox 权限了

@liuyib
Copy link
Collaborator

liuyib commented Feb 27, 2024

@hchlq 程哥,这个得你来看看了,我不清楚这块之前咋设计的。

目前的行为:

  1. 点击排序,数据中会多出 filters, sorter, extra 三个字段
    image
  2. 点击 reset,上面三个字段又没了(即恢复到了页面最初进来的样子)
    image

这个 PR 修改后的行为:

  1. 点击排序,数据中会多出 filters, sorter, extra 三个字段(同上)
  2. 点击 reset,上面三个字段仍然保留
    image

所以,reset 的时候,需要重置 filters, sorter, extra 吗?

@hchlq
Copy link
Collaborator

hchlq commented Feb 28, 2024

龙哥之前是这么设计的,也不太确定之前的上下文,不过 reset 的定义本身就是重置表单,这么改可能有 breakchange。

另外不改会有什么问题?

@liuyib liuyib removed the 🤔 Need Reproduce We cannot reproduce your problem label Feb 28, 2024
@hchlq
Copy link
Collaborator

hchlq commented Feb 28, 2024

@guaijie 对于你提的问题,重置之后当下的代码不是重置 defaultParams[0] 了吗,不是包含了 sorter,filter 相关字段了吗

_submit({
      ...(defaultParams?.[0] || {}),
      pageSize: options.defaultPageSize || options.defaultParams?.[0]?.pageSize || 10,
      current: 1,
    });

defaultParams[0] 即为分页数据

分页数据 { current, pageSize, sorter, filters, extra }

@liuyib
Copy link
Collaborator

liuyib commented Feb 28, 2024

不过 reset 的定义本身就是重置表单

既然这样,是不是不应该重置 table 的参数比如 sorter。目前 reset 会把 table 筛选带的参数重置掉,也就是影响到 table 主体了。

@hchlq
Copy link
Collaborator

hchlq commented Feb 28, 2024

但是我仔细看了下文档,应该是文档写的不是特别严谨,是要重置 Table 相关数据的

@hchlq
Copy link
Collaborator

hchlq commented Feb 28, 2024

当下的表现是重置 defaultParams[0] 和 pageSize 相关数据,因为没有传入 sorter 和 filter 相关信息,因此不会重置,符合当时设计的预期。

@hchlq
Copy link
Collaborator

hchlq commented Feb 28, 2024

如果要改的话,应该是增加 sorter 或者 filter 的默认值,但是感觉不是很有必要捏

@liuyib
Copy link
Collaborator

liuyib commented Feb 28, 2024

@hchlq 明白了程哥,你说的没错。

那你再看下这个行为:

先点击列进行排序操作,然后重置,排序操作的参数丢失了,重新点击分页,排序的字段又出现了

是不是有点问题~

@guaijie
Copy link
Contributor Author

guaijie commented Feb 28, 2024

如果要改的话,应该是增加 sorter 或者 filter 的默认值,但是感觉不是很有必要捏

抱歉,我前面表述有误,我似乎弄混淆了😅。我开始想表达的是表格也被表单重置影响了,因为按道理重置只影响表单,然后由用户确定是否影响表格 (也就是传参给 defaultParams)。这就是为什要加 params 的原因。但作者似乎默认了重置表格这一行为,如果也会重置表格,那就要考虑要不要重置每页条数也就是 pageSize ,通常我们肯定不希望重置 pageSize ,否则用户总是在重置表单后,又得调整它的大小(或者说用户不想每次重置表单后重置 pageSize 该怎么办,重置表单和重置表格并未分离)。但如果不重置 pageSize 那也就表示并没有完全重置表格,出现了和描述不一致的情况。

在不改动原有逻辑的情况下,以下是我的几点对现有代码的改动意见
image

// initPagination 命名并不不合适。最终参数应由外部传参决定,也就是 params
const _submit = (params: TParams[0]) => {
    if (!ready) {
      return;
    }
    setTimeout(() => {
      validateFields()
        .then((values = {}) => {
          const pagination =  {
            pageSize: 10, 
            current: 1,  // pageSize 和 current 在这里只作为容错(或者说给定的初始默认值)
             ...params,
          };
          if (!form) {
            // @ts-ignore
            run(pagination);
            return;
          }
          // record all form data
          allFormDataRef.current = {
            ...allFormDataRef.current,
            ...values,
          };
          // @ts-ignore
          run(pagination, values, {
            allFormData: allFormDataRef.current,
            type,
          });
        })
        .catch((err) => err);
    });
  };


  reset 代码中 _submit({ ...defaultParams?.[0] })

  submit 代码中 _submit(
      runSuccessRef.current
        ? { ...params?.[0], current: defaultParams?.[0]?.current ?? 1 }  // params 中包含所执行时所有参数,使用 ?? 代表即使用户设置 current 为 0 也行
        : { ...defaultParams?.[0] },
    );

上述代码将 defaultParams 作为 initParams 使用,符合作者预期。且代码更为简洁明确。如果用户有意设置 defaultParams.current 为 2,那就每次从第二页开始查,这是用户行为,即便出错。

所以根据我从源码所看到的和理解的,我会移除 defaultPageSize 然后为 defaultParams 中的每一参数赋初始值(上述容错的方式也可以),然后修改类型中的每一参数都是可选的。这样来统一默认传参方式。

我建议 defaultParams 是个对象而非数组,只用来配置表格参数,至于第二个参数本来就可以用表单的初始值来配。重置本来也没有用到第二个参数

简化掉复杂的参数处理。

调整下文档:reset 同样会重置表格相关参数。

此上。

不需要这个合并请求。

@Tz2318387771
Copy link

重置能把sorter状态清除,但是filters状态无法清除。这是bug吗?
screenshot-20240410-173632

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants