fix(form):fixed an issue where queryFilter controls were abnormally hidden

Lemonnnnnnnnnnn commented 3 months ago

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

github-actions[bot] commented 3 months ago

⚡️ Deploying PR Preview...

Lemonnnnnnnnnnn commented 3 months ago

我不是特别理解 defaultColsNumber 的预期行为，根据 https://github.com/ant-design/pro-components/issues/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 commented 2 months ago

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

Lemonnnnnnnnnnn commented 2 months ago

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

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

https://github.com/ant-design/pro-components/issues/5821 这个 issue 里你好像也是说你们内部不需要这个功能。我推测可能是后续拓展成现在的功能的。

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

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

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

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

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

chenshuai2144 commented 2 months ago

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

ant-design / pro-components

fix(form):fixed an issue where queryFilter controls were abnormally hidden #8564