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: improve upload component scrolling issue during form validation #45410

Closed
wants to merge 5 commits into from

Conversation

Wxh16144
Copy link
Member

@Wxh16144 Wxh16144 commented Oct 18, 2023

[中文版模板 / Chinese template]

🤔 This is a ...

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

🔗 Related issue link

fix: #28869

💡 Background and solution

一个历史遗漏问题, 表单滚动到错误位置需要在表单控件中绑定id, 但是上传组件的 id 在一个隐藏的 input 上,所以 api 无法得到元素渲染的具体位置。所以需要将收入控件渲染在 dom 中,然后用 css 内联样式隐藏起来。

如果表单中有上传组件,则推荐 scrollToFirstError 以对象方式设置元素块在视口中居中显示。

scrollToFirstError={{
  block: 'center',
}}

📝 Changelog

Language Changelog
🇺🇸 English improve upload component scrolling issue during form validation
🇨🇳 Chinese 修复上传组件在表单滑动至校验失败位置错误

☑️ Self-Check before Merge

⚠️ Please check all items below before requesting a reviewing. ⚠️

  • 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

🚀 Summary

🤖 Generated by Copilot at fcc79c3

This pull request updates the rc-upload dependency and modifies the Upload component to fix a Safari bug and improve the style consistency and flexibility. It changes the files components/upload/style/index.ts, components/upload/Upload.tsx, and package.json.

🔍 Walkthrough

🤖 Generated by Copilot at fcc79c3

  • Fix upload button not clickable on Safari by adding custom styles and classNames to rc-upload component (link, link)
  • Update rc-upload dependency version to 4.4.0 in package.json to use the latest bug fixes and improvements (link)

@stackblitz
Copy link

stackblitz bot commented Oct 18, 2023

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

@github-actions
Copy link
Contributor

github-actions bot commented Oct 18, 2023

Preview failed

@argos-ci
Copy link

argos-ci bot commented Oct 18, 2023

The latest updates on your projects. Learn more about Argos notifications ↗︎

Waiting for the first build to start…

@codecov
Copy link

codecov bot commented Oct 18, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (e665894) to head (2dbe65e).

Additional details and impacted files
@@            Coverage Diff            @@
##            master    #45410   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          743       743           
  Lines        12856     12854    -2     
  Branches      3364      3363    -1     
=========================================
- Hits         12856     12854    -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@@ -350,6 +350,12 @@ const InternalUpload: React.ForwardRefRenderFunction<UploadRef, UploadProps> = (
disabled: mergedDisabled,
beforeUpload: mergedBeforeUpload,
onChange: undefined,
styles: {
input: { display: 'unset', visibility: 'hidden' },
Copy link
Member

Choose a reason for hiding this comment

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

这两个样式为啥不放 style/index.ts 里面?

Copy link
Member Author

Choose a reason for hiding this comment

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

rc-upload 里面默认存在 display none 逻辑,https://github.com/react-component/upload/pull/518/files

Copy link
Member

Choose a reason for hiding this comment

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

那要不要改 rc-upload?rc-upload 再加个滚动 demo

Copy link
Member Author

Choose a reason for hiding this comment

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

rc-upload 没有form 滚动错误的操作。 验证这个 PR 估计得本地验证了。从 issue copy 了一份可以复现代码

import React from 'react';
import {
  Form,
  Select,
  InputNumber,
  Switch,
  Radio,
  Slider,
  Button,
  Upload,
  Rate,
} from 'antd';
import { UploadOutlined, InboxOutlined } from '@ant-design/icons';

const { Option } = Select;
const formItemLayout = {
  labelCol: {
    span: 6,
  },
  wrapperCol: {
    span: 14,
  },
};

const MUpload = (props) => {
  const { id, ..._props } = props;

  return (
    <span id={id}>
      <Upload {..._props}>{props.children}</Upload>
    </span>
  );
};

const normFile = (e) => {
  console.log('Upload event:', e);

  if (Array.isArray(e)) {
    return e;
  }

  return e && e.fileList;
};

const Demo = () => {
  const onFinish = (values) => {
    console.log('Received values of form: ', values);
  };

  return (
    <Form
      {...formItemLayout}
      onFinish={onFinish}
scrollToFirstError={{
  block: 'center',
}}
      initialValues={{
        'input-number': 3,
        'checkbox-group': ['A', 'B'],
        rate: 3.5,
      }}
    >

      <Form.Item
        wrapperCol={{
          span: 12,
          offset: 6,
        }}
      >
        <Button type="primary" htmlType="submit">
          Submit start
        </Button>
      </Form.Item>

      <br />
      <Form.Item label="Plain Text">
        <span className="ant-form-text">China</span>
      </Form.Item>
      <Form.Item
        name="select"
        label="Select"
        hasFeedback
      // rules={[
      //   {
      //     required: true,
      //     message: 'Please select your country!',
      //   },
      // ]}
      >
        <Select placeholder="Please select a country">
          <Option value="china">China</Option>
          <Option value="usa">U.S.A</Option>
        </Select>
      </Form.Item>

      <Form.Item
        name="upload"
        label="Upload"
        valuePropName="fileList"
        getValueFromEvent={normFile}
        rules={[
          {
            required: true,
            message: 'Please select your country!',
          },
        ]}
        extra="longgggggggggggggggggggggggggggggggggg"
      >
        <MUpload id="upload" name="logo" action="/upload.do" listType="picture">
          <Button>
            <UploadOutlined />
            点击上传
          </Button>
        </MUpload>
      </Form.Item>

      <Form.Item
        name="select-multiple"
        label="Select[multiple]"
      // rules={[
      //   {
      //     required: true,
      //     message: 'Please select your favourite colors!',
      //     type: 'array',
      //   },
      // ]}
      >
        <Select mode="multiple" placeholder="Please select favourite colors">
          <Option value="red">Red</Option>
          <Option value="green">Green</Option>
          <Option value="blue">Blue</Option>
        </Select>
      </Form.Item>

      <Form.Item label="InputNumber">
        <Form.Item name="input-number" noStyle>
          <InputNumber min={1} max={10} />
        </Form.Item>
        <span className="ant-form-text"> machines</span>
      </Form.Item>

      <Form.Item name="switch" label="Switch" valuePropName="checked">
        <Switch />
      </Form.Item>

      <Form.Item name="slider" label="Slider">
        <Slider
          marks={{
            0: 'A',
            20: 'B',
            40: 'C',
            60: 'D',
            80: 'E',
            100: 'F',
          }}
        />
      </Form.Item>

      <Form.Item name="radio-group" label="Radio.Group">
        <Radio.Group>
          <Radio value="a">item 1</Radio>
          <Radio value="b">item 2</Radio>
          <Radio value="c">item 3</Radio>
        </Radio.Group>
      </Form.Item>

      <Form.Item
        name="radio-button"
        label="Radio.Button"
      // rules={[
      //   {
      //     required: true,
      //     message: 'Please pick an item!',
      //   },
      // ]}
      >
        <Radio.Group>
          <Radio.Button value="a">item 1</Radio.Button>
          <Radio.Button value="b">item 2</Radio.Button>
          <Radio.Button value="c">item 3</Radio.Button>
        </Radio.Group>
      </Form.Item>

      <Form.Item name="rate" label="Rate">
        <Rate />
      </Form.Item>

      <Form.Item label="Dragger">
        <Form.Item
          name="dragger"
          valuePropName="fileList"
          getValueFromEvent={normFile}
          noStyle

          rules={[
            {
              required: true,
              message: 'Please select your country!',
            },
          ]}
        >
          <Upload.Dragger name="files" action="/upload.do">
            <p className="ant-upload-drag-icon">
              <InboxOutlined />
            </p>
            <p className="ant-upload-text">
              Click or drag file to this area to upload
            </p>
            <p className="ant-upload-hint">
              Support for a single or bulk upload.
            </p>
          </Upload.Dragger>
        </Form.Item>

      </Form.Item>
      <Form.Item label="Plain Text">
          <span style={{ display: 'inline-block', height: 200 }} className="ant-form-text">China</span>
        </Form.Item>

        <Form.Item
        wrapperCol={{
          span: 12,
          offset: 6,
        }}
      >
        <Button type="primary" htmlType="submit">
          Submit end
        </Button>
      </Form.Item>
    </Form>
  );
};

export default Demo;

Copy link
Member

Choose a reason for hiding this comment

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

不一定是 Form 的 scrollToFirstError,可以自己写个滚动的方法,验证

Copy link
Member Author

Choose a reason for hiding this comment

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

滚动没啥问题,这个 pr 解决的还是 upload 的 input 是一个 display: none 的问题

@Wxh16144 Wxh16144 marked this pull request as draft October 20, 2023 10:35
Copy link
Contributor

github-actions bot commented Mar 31, 2024

👁 Visual Regression Report for PR #45410 Passed ✅

🎯 Target branch: master (e665894)
📖 View Full Report ↗︎

🎊 Congrats! No visual-regression diff found.

Copy link

codesandbox-ci bot commented Mar 31, 2024

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

@Wxh16144
Copy link
Member Author

'input[type="file"]': {
display: 'block',
},
这段代码应该可以删除了 @MadCcc review 一下? blame 是你2年前的提交

@Wxh16144 Wxh16144 marked this pull request as ready for review March 31, 2024 17:03
@Wxh16144
Copy link
Member Author

ready~

@Wxh16144 Wxh16144 requested a review from crazyair March 31, 2024 17:04
@Wxh16144

This comment was marked as off-topic.

@Wxh16144
Copy link
Member Author

Wxh16144 commented Apr 1, 2024

我估计的 Preview Deploy 挂了是因为之前的 surge key 部署的不能对其进行覆盖导致的🤔

@Wxh16144
Copy link
Member Author

Wxh16144 commented Apr 1, 2024

我看看 jsdom 能不能模拟滚动写个单元测试

@zombieJ
Copy link
Member

zombieJ commented Apr 1, 2024

感觉可以优化一下 Form 的逻辑,先看看子元素支不支持 ref,然后如果支持先用 ref 滚动,不行再 id 滚动

@Wxh16144
Copy link
Member Author

感觉可以优化一下 Form 的逻辑,先看看子元素支不支持 ref,然后如果支持先用 ref 滚动,不行再 id 滚动

已在 #48211 换方案修复~

@Wxh16144 Wxh16144 closed this Apr 18, 2024
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.

scrollToFirstError for Upload input
4 participants