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(form):fixed an issue where queryFilter controls were abnormally hidden #8564

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

Lemonnnnnnnnnnn
Copy link

fix(form):fixed an issue where queryFilter controls were abnormally hidden when defaultColsNumber were specified.

related issue: #8563

Copy link

github-actions bot commented Jul 13, 2024

⚡️ Deploying PR Preview...

@Lemonnnnnnnnnnn
Copy link
Author

Lemonnnnnnnnnnn commented Jul 17, 2024

我不是特别理解 defaultColsNumber 的预期行为,根据 #5328 来看,“defaultColsNumber : 6,预期应该显示5个控件”,但测试代码却表示 "defaultColsNumber: 5 应该显示 3 个控件" :

  it('🕵️‍♀️ defaultColsNumber should work', async () => {
    const { container } = render(
      <QueryFilter defaultColsNumber={5}>
        <ProFormText label="a" name="a" />
        <ProFormText label="b" name="b" />
        <ProFormText label="c" name="c" />
        <ProFormText label="d" name="d" />
        <ProFormText label="e" name="e" />
        <ProFormText label="f" name="f" />
      </QueryFilter>,
    );
    expect(
      container.querySelectorAll('.ant-row .ant-form-item-hidden'),
    ).toHaveLength(3);
  });

甚至“控件”的概念都有些模糊,官网的文档写着:“defaultColsNumber数量大于等于控件数量则隐藏展开按钮”,按照目前的实现效果来看,“控件”的概念是包括查询按钮在内的,而 issue 中称呼的“控件” 却又并非包括查询按钮。

@chenshuai2144
Copy link
Contributor

应该是包含查询按钮的,你要不要重写下这个逻辑

soul added 2 commits August 13, 2024 21:23
…ltFormItemsNumber to support the existing functionality of defaultColsNumber
@Lemonnnnnnnnnnn
Copy link
Author

Lemonnnnnnnnnnn commented Aug 13, 2024

我个人倾向于不包含查询按钮,比较符合直觉。。。 并且 defaultColsNumber 这个名字也很奇怪,看 TS 类型你们开始的时候似乎不打算考虑非折叠情况下展示多行的场景?

  /**
   * @name 默认一行显示几个表单项
  */
  defaultColsNumber?: number;

#5821 这个 issue 里你好像也是说你们内部不需要这个功能。我推测可能是后续拓展成现在的功能的。

如果是我改的话,我可能会拓展一个 defaultFormItemsNumber 字段,做现在 defaultColsNumber 做的事情,表单项个数 = n, defaultFormItemsNumber = m :

  • m <= n : 展示 m 个表单项
  • 否则展示 n 个表单项

defaultColsNumber 只处理单行的情况,表单项个数 = n , defaultColsNumber = m , 一行展示的表单项个数 = r:

  • m <= n && m <= r : 展示 m 个表单项
  • 否则展示 r 个表单项

我试着实现了一下,并修改了测试用例,但不知道你们能不能接受。。

@Lemonnnnnnnnnnn Lemonnnnnnnnnnn force-pushed the fix/form-QueryFilter-defaultColsNumber branch from 14068c0 to e6cd3f2 Compare August 13, 2024 15:34
@chenshuai2144
Copy link
Contributor

可以的,只要符合直觉就可以,之前的方法算的有点奇怪,一直不符合直觉

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.

2 participants