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.

Sign up for GitHub

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

Jump to bottom

feat: refactor editor image block upload logic #5159

Merged
merged 3 commits into from
Jan 19, 2024
Merged

feat: refactor editor image block upload logic #5159

merged 3 commits into from
Jan 19, 2024

Conversation

LIlGG
Copy link
Member

@LIlGG LIlGG commented Jan 9, 2024

What type of PR is this?

/kind feature

What this PR does / why we need it:

重构编辑器图片块上传逻辑，增加选择文件上传、上传进度条、取消、重试等机制。

How to test it?

直接拖动、复制或选择文件上传，查看是否显示上传进度条，取消、重试功能是否正常

Which issue(s) this PR fixes:

Fixes #5122

Does this PR introduce a user-facing change?

重构编辑器图片块上传逻辑，增加选择文件上传、上传进度条、取消、重试等机制。

@f2c-ci-robot f2c-ci-robot bot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. release-note Denotes a PR that will be considered when it comes time to generate release notes. kind/feature Categorizes issue or PR as related to a new feature. labels Jan 9, 2024
@f2c-ci-robot f2c-ci-robot bot requested review from JohnNiang and wzrove January 9, 2024 08:47
@codecov Codecov
Copy link

codecov bot commented Jan 9, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (df8bb33) 57.06% compared to head (2262b3c) 57.06%.

Additional details and impacted files 
@@            Coverage Diff            @@
##               main    #5159   +/-   ##
=========================================
  Coverage     57.06%   57.06%           
  Complexity     3333     3333           
=========================================
  Files           584      584           
  Lines         19190    19190           
  Branches       1444     1444           
=========================================
  Hits          10950    10950           
  Misses         7664     7664           
  Partials        576      576

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

@ruibaby
Copy link
Member

ruibaby commented Jan 10, 2024

/area editor
/milestone 2.12.x

@f2c-ci-robot f2c-ci-robot bot added this to the 2.12.x milestone Jan 10, 2024
@f2c-ci-robot f2c-ci-robot bot added the area/editor Issues or PRs related to the Editor label Jan 10, 2024
@LIlGG LIlGG changed the title WIP: feat: refactor editor image block upload logic feat: refactor editor image block upload logic Jan 11, 2024
@f2c-ci-robot f2c-ci-robot bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 11, 2024
@ruibaby
Copy link
Member

ruibaby commented Jan 11, 2024

测试之后发现几个问题：

  1. 点击整个卡片也可以调出上传弹框。

    image

    原因可能是因为卡片外层使用了 label 标签。

  2. 从附件库选择可以选择所有类型文件，建议仅可选择图片类型。

  3. 建议支持替换图片，点击替换图片按钮后，显示初始卡片，可以选择重新上传或者从附件库选择。如果用户有换图片的需求，这将减少很多操作步骤。

ruibaby
ruibaby requested changes Jan 11, 2024
View reviewed changes
console/packages/editor/src/components/index.ts Outdated Show resolved Hide resolved
console/src/components/editor/composables/use-attachment.ts Outdated Show resolved Hide resolved
console/src/components/editor/extensions/image/index.ts Outdated Show resolved Hide resolved
console/src/components/editor/extensions/image/index.ts Outdated Show resolved Hide resolved
console/src/components/editor/utils/attachment.ts Outdated Show resolved Hide resolved
console/src/components/editor/utils/upload.ts Outdated Show resolved Hide resolved
console/src/components/editor/extensions/image/ImageView.vue Outdated Show resolved Hide resolved
console/src/components/editor/extensions/image/ImageView.vue Outdated Show resolved Hide resolved
JohnNiang
JohnNiang reviewed Jan 11, 2024
View reviewed changes
Copy link
Member

@JohnNiang JohnNiang left a comment

Choose a reason for hiding this comment

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

上传图片后一直显示上传中，实际上图片已经正常上传至附件库中了。

image

我的环境：Windows11，Edge。

@LIlGG
Copy link
Member Author

LIlGG commented Jan 11, 2024

上传图片后一直显示上传中，实际上图片已经正常上传至附件库中了。
我的环境：Windows11，Edge。

我在 Windows 11 下使用开发环境运行此 PR，并且使用 Edge 120.0.2210.121 (正式版本) (64 位) 进行图片上传测试，并没有能够成功复现。

在你的环境下，该现象是偶现还是必现？或者是否还有什么其他我遗漏的操作？

guqing
guqing reviewed Jan 12, 2024
View reviewed changes
Copy link
Member

@guqing guqing left a comment

Choose a reason for hiding this comment

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

出现点击附件库没有反应的情况，好像是删除其中的框会复现
image

@guqing
Copy link
Member

guqing commented Jan 12, 2024

请看 vcr

Kapture.2024-01-12.at.17.16.13.mp4

展示了两个问题：

  1. 删除空白的附件占位符会错误的将正在上传的删除掉(或是被 cancel 掉不确定)
  2. 导致其他占位符点击附件库没有反应

@LIlGG
Copy link
Member Author

LIlGG commented Jan 15, 2024

  1. 删除空白的附件占位符会错误的将正在上传的删除掉(或是被 cancel 掉不确定)
  2. 导致其他占位符点击附件库没有反应

这是由于删除了 node 之后，再次设置此 attr 导致的报错。已修复

@ruibaby
Copy link
Member

ruibaby commented Jan 17, 2024 •
edited
Loading

图片

这个替换图片的图标给人一种重试的感觉。建议将替换按钮放置在任意一个角落即可，鼠标放上去也不需要大范围的效果。

或者放置在 Bubble Menu。

ruibaby
ruibaby approved these changes Jan 19, 2024
View reviewed changes
Copy link
Member

@ruibaby ruibaby left a comment

Choose a reason for hiding this comment

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

/approve

@f2c-ci-robot f2c-ci-robot bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 19, 2024
@LIlGG LIlGG force-pushed the feat/refactor-editor-upload branch from 6432db8 to 288b382 Compare January 19, 2024 08:19
@JohnNiang
Copy link
Member

上传图片后一直显示上传中，实际上图片已经正常上传至附件库中了。
我的环境：Windows11，Edge。

我在 Windows 11 下使用开发环境运行此 PR，并且使用 Edge 120.0.2210.121 (正式版本) (64 位) 进行图片上传测试，并没有能够成功复现。

在你的环境下，该现象是偶现还是必现？或者是否还有什么其他我遗漏的操作？

无论是在 Windows、Linux 还是在 macOS build console 之后必定复现该问题。

@guqing
Copy link
Member

guqing commented Jan 19, 2024

/hold

@f2c-ci-robot f2c-ci-robot bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 19, 2024
@guqing
Copy link
Member

guqing commented Jan 19, 2024

/unhold
/lgtm

@f2c-ci-robot f2c-ci-robot bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 19, 2024
@f2c-ci-robot f2c-ci-robot bot added the lgtm Indicates that a PR is ready to be merged. label Jan 19, 2024
@f2c-ci-robot f2c-ci-robot bot removed the lgtm Indicates that a PR is ready to be merged. label Jan 19, 2024
JohnNiang
JohnNiang approved these changes Jan 19, 2024
View reviewed changes
Copy link
Member

@JohnNiang JohnNiang left a comment

Choose a reason for hiding this comment

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

/approve

@f2c-ci-robot f2c-ci-robot
Copy link

f2c-ci-robot bot commented Jan 19, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: JohnNiang, ruibaby

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@JohnNiang
Copy link
Member

/lgtm

@f2c-ci-robot f2c-ci-robot bot added the lgtm Indicates that a PR is ready to be merged. label Jan 19, 2024
@JohnNiang JohnNiang merged commit 14580b9 into halo-dev:main Jan 19, 2024
6 of 7 checks passed
@JohnNiang JohnNiang modified the milestones: 2.12.x, 2.12.0 Jan 19, 2024
This was referenced Jan 24, 2024
Closed
Closed
@LIlGG LIlGG deleted the feat/refactor-editor-upload branch January 30, 2024 08:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@guqing guqing guqing left review comments

@ruibaby ruibaby ruibaby approved these changes

@JohnNiang JohnNiang JohnNiang approved these changes

@wzrove wzrove Awaiting requested review from wzrove

Assignees

@JohnNiang JohnNiang

@guqing guqing

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/editor Issues or PRs related to the Editor kind/feature Categorizes issue or PR as related to a new feature. lgtm Indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes.
Projects
None yet
Milestone
2.12.0
Development

Successfully merging this pull request may close these issues.

期望编辑器的图片块支持上传文件
4 participants
@LIlGG @ruibaby @guqing @JohnNiang