feat(ImageUploader): add another layout mode by `grid` #5728

MuxinFeng · 2022-10-11T10:10:40Z

relate issue

本次 PR 内容：

支持 grid 布局（参考 Selector 实现），其中单个图片组件的尺寸通过 column 和 gap 计算得出
添加了 grid 的基本的样式 props
添加了中英文参数说明
添加了一个 demo

github-actions · 2022-10-11T10:14:33Z

PR preview has been successfully built and deployed to https://antd-mobile-preview-pr-5728.surge.sh

codecov · 2022-10-11T10:15:04Z

Codecov Report

Base: 89.52% // Head: 90.05% // Increases project coverage by +0.53% 🎉

Coverage data is based on head (585c493) compared to base (678ca0a).
Patch coverage: 79.41% of modified lines in pull request are covered.

Additional details and impacted files

@@             Coverage Diff             @@
##           feature    #5728      +/-   ##
===========================================
+ Coverage    89.52%   90.05%   +0.53%     
===========================================
  Files          297      297              
  Lines         6375     6408      +33     
  Branches      1566     1582      +16     
===========================================
+ Hits          5707     5771      +64     
+ Misses         614      584      -30     
+ Partials        54       53       -1

Impacted Files	Coverage Δ
src/components/image-uploader/image-uploader.tsx	`86.56% <79.41%> (-1.93%)`	⬇️
src/components/cascader-view/cascader-view.tsx	`91.30% <0.00%> (+0.67%)`	⬆️
src/utils/matrix.ts	`94.73% <0.00%> (+26.31%)`	⬆️
src/components/image-viewer/slide.tsx	`58.40% <0.00%> (+31.99%)`	⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

zqran · 2022-10-13T02:47:26Z

src/components/image-uploader/index.en.md

@@ -15,7 +15,9 @@
 | accept              | The file types, `image/gif` `image/jpeg` `image/jpg` `image/png`                                                                                                         | `string`                                                                                                   | `image/*` |
 | beforeUpload        | Callback function before file reading, return `null` to terminate file reading, support return `Promise`                                                                 | `(file: File, files: File[]) => Promise<File \| null> \| File \| null`                                     | -         |
 | capture             | Picture selection mode, the optional value is `camera` (directly adjust the camera)                                                                                      | `boolean \| string`                                                                                        | -         |
-| children            | Custom upload button                                                                                                                                                     | `ReactNode`                                                                                                | -         |
+| children            | Custom upload button                                                                                                                                                     |
+| columns             | 行展示数                                                                                                                                                                 | `number`                                                                                                   | -         |


这里应该是英文描述吧？

嗯嗯，我改下哈～

zqran · 2022-10-13T02:52:26Z

src/components/image-uploader/image-uploader.less

@@ -2,6 +2,20 @@

 .@{class-prefix-image-uploader} {
  --cell-size: 80px;
+  --gap: 12px;
+  ---gap: var(--gap);


第一眼看到 ---gap 的写法感觉有些奇怪，误以为语法错误😅~~

这个是按照 selector 那边的写法，大佬能给点正确的写法的 demo 或者关键词吗

我有一个想法：就是直接去掉该组件的根元素 <div className={classPrefix}>，直接用 Grid 或 Space 作为该组件的根节点。这样就不需要 ---gap 这种额外的 CSS 变量了，这边的样式会简化很多。不知道去掉现在的根元素，会不会导致额外的问题。

我有两个想法哈，求大佬给指点一下 @zqran

一般组件的根节点的 className 一般都是组件名开头的，这算是个样式命名规范吧。

如果没有根节点限定范围，我担心会跟其他用Grid或Space组件的地方产生样式冲突（污染）。

我懂您意思了，我这边试试看是否会有意外影响

改好了哈，目前没有发现额外的问题，麻烦大佬再看看~

zqran · 2022-10-18T05:49:31Z

src/components/image-uploader/image-uploader.tsx

+  return withNativeProps(
+    props,
+    props.columns ? (
+      <Grid className={`${classPrefix}-grid`} columns={props.columns}>


这边缺少 classPrefix 类名，导致单测失败了

目前在run test过程中发现两个问题：

删除了原来的根节点，造成image-uploader的测试快照不一致，已通过npm test -- -u生成新的快照

image-viewer 有个测试用例没过，初步判断是偏移量有问题，这个我需要再看一下 jest 相关的文档再尝试修复。

对于问题1，我觉得应该保留 classPrefix 类名，这是 antd-mobile 组件库的规范，所有组件都自己的根类名

那这样的话，还是要加上原来的根节点吧

可以保留类名，类似：${classPrefix} ${classPrefix}-grid 这种形式~

嗷嗷嗷嗷，恍然大悟

miracles1919 · 2022-10-18T07:49:47Z

src/components/image-uploader/image-uploader.less

+  &-grid {
+    --gap: 12px;
+    --cell-size: calc(
+      (100vw - var(--gap) * (var(--columns) - 1)) / var(--columns)


100vw 是有问题的

这个地方应该用父级元素的宽度来计算吧，也就是 100% 替换 100vw，但是我这样改组件高度直接没有了，大佬有什么想法吗

试下用 js 计算

我试试哈

miracles1919 · 2022-10-18T07:50:16Z

src/components/image-uploader/index.en.md

@@ -58,8 +59,12 @@

 ## FAQ

-### The `capture` attribute is configured, but why some `Android` phones will still have the file option?
+### 1. The `capture` attribute is configured, but why some `Android` phones will still have the file option?


Suggested change

### 1. The `capture` attribute is configured, but why some `Android` phones will still have the file option?

### The `capture` attribute is configured, but why some `Android` phones will still have the file option?

miracles1919 · 2022-10-18T07:50:40Z

src/components/image-uploader/index.en.md


 The `capture` provided by ImageUploader comes from the native HTML capability, and in some operating systems/browser environments, this attribute may not be supported, so this problem cannot be avoided.

 See the discussion in this [issue](https://github.com/ant-design/ant-design-mobile/issues/5254).
+
+### 2. Compatibility description of the `columns` attribute


Suggested change

### 2. Compatibility description of the `columns` attribute

### Compatibility description of the `columns` attribute

不用加序号

MuxinFeng · 2022-10-19T11:58:13Z

本次 PR 难点在于无法得出栅格布局中的每个单元组件的属性，目前是通过 useSize 拿到父元素的宽度再计算得出。 @miracles1919

目前 code review 发现的问题已解决，实现过程中的一点疑问跟大佬们沟通下：

在 space 和 grid 之外加上了根节点（也是其他组件的一贯风格>.<），这会造成样式看起来稍微有点繁琐，但如果不加这个根元素，就无法通过 useSize 获取容器的宽度； @zqran
最终渲染组件时，传入 props 的方式没那么优雅。之前试过在 useEffect 里更改 props.style ，但是触发了重新渲染后，props 又重新变成默认值const props = mergeProps(defaultProps, p)。后来又想到用 state 存储 props，但这样又会造成图片最后一张无法删除的问题，所以最终采取了这种写法。

miracles1919

cell-size 的大小除了跟 container 相关外，--gap-horizontal 和 --cell-size 应该是关联的：

改了 --cell-size, --gap 就要变化
改了 --gap, --cell-size 就要变化

miracles1919 · 2022-10-20T12:43:28Z

src/components/image-uploader/image-uploader.tsx

+      const width = containerSize.width
+      const columns = props.columns
+      const gap =
+        Number(props.style?.['--gap-horizontal']?.replace('px', '')) || 12


css 的单位不只是 px，可能还有 rem vw，可以用 measureCSSLength，参考：https://github.com/ant-design/ant-design-mobile/blob/master/src/components/picker-view/wheel.tsx#L53

这里我改一下

miracles1919 · 2022-10-20T12:48:59Z

src/components/image-uploader/image-uploader.tsx

+    { ...props, ...extraStyle },
+    <div className={classPrefix} ref={containerRef}>


Suggested change

{ ...props, ...extraStyle },

<div className={classPrefix} ref={containerRef}>

<div className={classPrefix} ref={containerRef} style={style}>

其实这里的 extraStyle 主要是一个 --cell-size 属性，放在 style 里会有类型错误哈

所以 style 需要额外的 ts 定义，ref：https://github.com/ant-design/ant-design-mobile/blob/master/src/components/tag/tag.tsx#L42

MuxinFeng · 2022-10-21T02:01:45Z

cell-size 的大小除了跟 container 相关外，--gap-horizontal 和 --cell-size 应该是关联的：

改了 --cell-size, --gap 就要变化

改了 --gap, --cell-size 就要变化

我觉得在 grid 布局下，就不能指定 --cell-size 了，因为容器的宽度一定，--gap-horizontal 也是一定的（要么指定，要么默认用 --gap），这样里面的--cell-size 也就固定下来了

MuxinFeng · 2022-10-21T03:42:51Z

done

miracles1919 · 2022-10-24T04:04:08Z

在你的基础上调整了一下

--cell-size 的计算逻辑
--gap 两种布局都能支持，所以就不限制了
更新了下文档

MuxinFeng · 2022-10-24T04:51:29Z

在你的基础上调整了一下

--cell-size 的计算逻辑

--gap 两种布局都能支持，所以就不限制了

更新了下文档

感谢大佬，get 很多新知识

zombieJ · 2022-10-27T09:10:12Z

src/components/image-uploader/demos/demo2.tsx

@@ -59,6 +80,10 @@ export default () => {
        <CustomeSize />
      </DemoBlock>

+      <DemoBlock title='自定义列数'>


这边加一下描述，如果设置了 columns，cell size 会失效，而会根据空间自动变化。

这个在 FAQ 里加了哈，这是是要重复强调一下吗

zombieJ · 2022-10-27T09:41:58Z

src/components/image-uploader/image-uploader.tsx

-    props,
-    <div className={classPrefix}>
-      <Space className={`${classPrefix}-space`} wrap block>
+  const renderChildren = () => {


这边看起来不需要做成 function，直接：

const contentNode = ( <> {...} </> );

下面直接：

<Grid ...> ... {contentNode} </Grid>

zombieJ · 2022-10-27T09:42:50Z

src/components/image-uploader/index.zh.md

+
+### `columns` 属性说明
+
+`columns` 属性依赖 [Grid](./grid) 布局，该属性存在时，不支持自定义 `--cell-size` 属性，因为图片和上传按钮的大小是自动计算的。


上面 Demo 也需要注明一下，不用看到的这么远的描述。

MuxinFeng · 2022-10-27T11:02:35Z

done @zombieJ

awmleer · 2022-10-27T15:17:38Z

src/components/image-uploader/index.zh.md

+| --cell-size      | 图片和上传按钮的大小 | `80px`       |
+| --gap            | 间距大小             | `12px`       |
+| --gap-horizontal | 水平方向的间距大小   | `var(--gap)` |
+| --gap-vertical   | 垂直方向的间距大小   | `var(--gap)` |


@awmleer 这里我调整了一下，Space 也能支持 gap

https://github.com/ant-design/ant-design-mobile/pull/5728/files#diff-5a0391155d29fdbc3ab571bf6c9500de0e42125489e5f1ec6bfffe76dd42c62aR11-R15

Co-authored-by: 卡晨 <awmleer@icloud.com>

MuxinFeng · 2022-10-27T15:31:06Z

done @awmleer 大佬辛苦啦

This reverts commit 71c8bcb.

This reverts commit d673d44.

MuxinFeng · 2022-10-31T02:43:48Z

应该只需要改下 --cell-size 的说明哈，commit 已提交 @miracles1919 @awmleer

zombieJ · 2022-10-31T09:52:51Z

conflict 了

MuxinFeng · 2022-10-31T11:43:56Z

conflict 了

好啦，麻烦大佬再看看

jlala · 2022-11-01T03:14:50Z

正好也给了我点思路幸运

zombieJ · 2022-11-02T09:30:59Z

approved. Base 改一下到 feature 哈~

MuxinFeng · 2022-11-02T09:33:38Z

approved. Base 改一下到 feature 哈~

rebase 一下 feature 分支是吧

MuxinFeng · 2022-11-02T09:58:17Z

approved. Base 改一下到 feature 哈~

才反应过来，done 啦

src/components/image-uploader/index.zh.md

Co-authored-by: zqran <215244947@qq.com>

resolved

miracles1919 · 2022-11-07T15:14:14Z

合并了，辛苦了 ~

feat(ImageUploader): add and other css props

d948bfd

MuxinFeng changed the title ~~feat(ImageUploader): add and other css props~~ feat(ImageUploader): add columns and other css props Oct 12, 2022

zqran requested changes Oct 13, 2022

View reviewed changes

zqran force-pushed the feat-image-uploader branch from 93f479c to d948bfd Compare October 13, 2022 03:15

fengxin added 2 commits October 14, 2022 15:43

feat(ImageUploader): add FAQ about grid

afc2bbe

feat(ImageUploader): remove redundant code

aa3d209

zqran requested changes Oct 18, 2022

View reviewed changes

miracles1919 reviewed Oct 18, 2022

View reviewed changes

feat(ImageUploader): calc cell-size by js & remove serial number in FAQ

ef71212

MuxinFeng changed the title ~~feat(ImageUploader): add columns and other css props~~ feat(ImageUploader): add another layout mode by grid Oct 20, 2022

miracles1919 reviewed Oct 20, 2022

View reviewed changes

feat(image-uploader): optimize the calculation method of --cell-size

387af1e

miracles1919 added 2 commits October 24, 2022 11:42

refactor: adjust cell size calculation logic

29c1ad1

feat: Space layout support CSS Variables --gap

291e1a1

miracles1919 requested review from awmleer, zqran and zombieJ October 24, 2022 06:13

zombieJ reviewed Oct 27, 2022

View reviewed changes

feat(ImageUploader): optimize code

658317c

awmleer reviewed Oct 27, 2022

View reviewed changes

MuxinFeng and others added 2 commits October 27, 2022 23:23

feat(ImageUploader): fix description

d673d44

Co-authored-by: 卡晨 <awmleer@icloud.com>

feat(ImageUploader): optimize description en

71c8bcb

fengxin added 3 commits October 31, 2022 10:37

Revert "feat(ImageUploader): optimize description en"

7470619

This reverts commit 71c8bcb.

Revert "feat(ImageUploader): fix description"

26f8b3f

This reverts commit d673d44.

feat: fix description

c1bde5d

Merge branch 'master' into feat-image-uploader

9ebc95b

zombieJ approved these changes Nov 2, 2022

View reviewed changes

MuxinFeng changed the base branch from master to feature November 2, 2022 09:57

zqran previously requested changes Nov 3, 2022

View reviewed changes

src/components/image-uploader/index.zh.md Outdated Show resolved Hide resolved

fix: fix typo

585c493

Co-authored-by: zqran <215244947@qq.com>

MuxinFeng mentioned this pull request Nov 3, 2022

doc(Selector): fix typo #5784

Merged

miracles1919 enabled auto-merge (squash) November 7, 2022 15:10

miracles1919 approved these changes Nov 7, 2022

View reviewed changes

miracles1919 merged commit 3b3eb4b into ant-design:feature Nov 7, 2022

	### 1. The `capture` attribute is configured, but why some `Android` phones will still have the file option?
	### The `capture` attribute is configured, but why some `Android` phones will still have the file option?

	### 2. Compatibility description of the `columns` attribute
	### Compatibility description of the `columns` attribute

		{ ...props, ...extraStyle },
		<div className={classPrefix} ref={containerRef}>


		### `columns` 属性说明

		`columns` 属性依赖 [Grid](./grid) 布局，该属性存在时，不支持自定义 `--cell-size` 属性，因为图片和上传按钮的大小是自动计算的。

feat(ImageUploader): add another layout mode by grid #5728

feat(ImageUploader): add another layout mode by grid #5728

Conversation

MuxinFeng commented Oct 11, 2022

github-actions bot commented Oct 11, 2022

codecov bot commented Oct 11, 2022 • edited

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MuxinFeng commented Oct 19, 2022

miracles1919 left a comment • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MuxinFeng commented Oct 21, 2022

MuxinFeng commented Oct 21, 2022

miracles1919 commented Oct 24, 2022

MuxinFeng commented Oct 24, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MuxinFeng commented Oct 27, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MuxinFeng commented Oct 27, 2022

MuxinFeng commented Oct 31, 2022

zombieJ commented Oct 31, 2022

MuxinFeng commented Oct 31, 2022 • edited

jlala commented Nov 1, 2022

zombieJ commented Nov 2, 2022

MuxinFeng commented Nov 2, 2022

MuxinFeng commented Nov 2, 2022

miracles1919 commented Nov 7, 2022

feat(ImageUploader): add another layout mode by `grid` #5728

feat(ImageUploader): add another layout mode by `grid` #5728

codecov bot commented Oct 11, 2022 •

edited

miracles1919 left a comment •

edited

MuxinFeng commented Oct 31, 2022 •

edited