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: Backspace does not work properly behind list #5102

Merged
merged 5 commits into from
Jan 2, 2024

Conversation

AirboZH
Copy link
Member

@AirboZH AirboZH commented Dec 22, 2023

What type of PR is this?

/kind bug

What this PR does / why we need it:

增加 @tiptap/extension-list-keymap 扩展,优化列表的键盘操作

Which issue(s) this PR fixes:

Fixes #5065

Special notes for your reviewer:

测试方法:

  1. 测试无序列表中和无序列表后对于删除键 Delete 和退格键 backspace 的支持是否符合预期
  2. 测试有序列表中和有序列表后对于删除键 Delete 和退格键 backspace 的支持是否符合预期
    2023-12-22
  3. 测试选择部分列表项后对于删除键 Delete 和退格键 backspace 的支持是否符合预期
  4. 测试 Ctrl-A 全选后对于删除键 Delete 和退格键 backspace 的支持是否符合预期
    selectAll

Does this PR introduce a user-facing change?

修复编辑器中无法删除有序/无序列表后空行的问题

@f2c-ci-robot f2c-ci-robot bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. kind/bug Categorizes issue or PR as related to a bug. labels Dec 22, 2023
@f2c-ci-robot f2c-ci-robot bot requested review from LIlGG and wan92hen December 22, 2023 07:32
Copy link

codecov bot commented Dec 22, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (36ebc24) 55.91% compared to head (6b8f07a) 55.91%.
Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff            @@
##               main    #5102   +/-   ##
=========================================
  Coverage     55.91%   55.91%           
  Complexity     3032     3032           
=========================================
  Files           524      524           
  Lines         17826    17826           
  Branches       1329     1329           
=========================================
  Hits           9968     9968           
  Misses         7309     7309           
  Partials        549      549           

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

Copy link
Member

@LIlGG LIlGG left a comment

Choose a reason for hiding this comment

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

hi, @AirboZH ,感谢提交的 PR。
经过测试,我发现此提交会对其他快捷键产生冲突。例如:

输入:

1. 123
2. 123

之后,按 Mod + a 进行全选,然后按删除键进行删除,此时只会删除 list item 的最后一个 item。
这应该是删除键与快捷键冲突导致,删除键遇到 item 的 li 标签后,将其还原为了普通文本就停止继续操作了。

@AirboZH
Copy link
Member Author

AirboZH commented Dec 22, 2023

经过测试,我发现此提交会对其他快捷键产生冲突。

🤨确实是我自测的时候没测到这个问题。

这是一个已知的问题,并且也有其他 tiptap下游软件受到影响。
ueberdosis/tiptap#4395
但是这其中的 PR 和 Issue 好像很久都没有人处理了。

我们可以等待上游处理。

同时,我也在看看有没有机会在 list-keymap extension 的基础上处理一下全选删除这个操作。

@LIlGG
Copy link
Member

LIlGG commented Dec 22, 2023

同时,我也在看看有没有机会在 list-keymap extension 的基础上处理一下全选删除这个操作。

tiptap 在写这个扩展的时候,将工具类 list-keymap 也导出了,那么只需要在 Halo 中重构扩展逻辑即可。

可以提供一种思路,例如在 Backspace 操作执行其余操作之前,先校验 selection 是否完全包含了当前 node 列表,如果包含,则执行 delete node 即可。

@LIlGG
Copy link
Member

LIlGG commented Dec 25, 2023

似乎还是有问题。当不是全选时,将会出现与之前同样的问题。

屏幕录制2023-12-25 12 26 22

@f2c-ci-robot f2c-ci-robot bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 28, 2023
@f2c-ci-robot f2c-ci-robot bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 29, 2023
Copy link
Member

@LIlGG LIlGG left a comment

Choose a reason for hiding this comment

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

/lgtm

nice,它可以正常工作,很完美。

@f2c-ci-robot f2c-ci-robot bot added the lgtm Indicates that a PR is ready to be merged. label Dec 29, 2023
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.

/lgtm

Copy link

f2c-ci-robot bot commented Jan 2, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: LIlGG, 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

@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 2, 2024
@f2c-ci-robot f2c-ci-robot bot merged commit c918f2c into halo-dev:main Jan 2, 2024
4 checks passed
@JohnNiang
Copy link
Member

/milestone 2.12.x

@f2c-ci-robot f2c-ci-robot bot added this to the 2.12.x milestone Jan 2, 2024
@JohnNiang JohnNiang modified the milestones: 2.12.x, 2.12.0 Jan 7, 2024
f2c-ci-robot bot pushed a commit to halo-sigs/plugin-moments that referenced this pull request Apr 11, 2024
…list (#92)

#### What type of PR is this?

/kind bug

#### What this PR does / why we need it:

增加 keymap 扩展,用于解决列表组件无法使用退格键删除的问题。

See halo-dev/halo#5102

#### How to test it?

测试列表能否使用退格键删除

#### Which issue(s) this PR fixes:

Fixes #91 

#### Does this PR introduce a user-facing change?
```release-note
修复瞬间编辑区域列表组件无法被删除的问题
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. kind/bug Categorizes issue or PR as related to a bug. 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
4 participants