-
Notifications
You must be signed in to change notification settings - Fork 38
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(table): replace deprecated with Clipboard API #139
Conversation
WalkthroughThe changes in this pull request involve modifications to the Changes
Possibly related PRs
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (4)
packages/fluent-editor/src/table/modules/table-operation-menu.ts (4)
85-95
: Remove unnecessary call tosetCopyRange(dom)
Since we're now using the Clipboard API to write content directly to the clipboard, the call to
this.setCopyRange(dom)
is redundant and can be removed.Apply this diff to remove the unnecessary call:
trArr.forEach(tr => tr.removeAttribute('data-row')) - this.setCopyRange(dom) const blob = new Blob([dom.outerHTML], { type: 'text/html' }) const clipboardItem = new ClipboardItem({ 'text/html': blob }) await navigator.clipboard.write([clipboardItem])
Line range hint
102-104
: UpdatecopyCells
handler to be asynchronousSince
onCopy
is now an asynchronous function, thecopyCells
handler should be declared asasync
and await the call tothis.onCopy('copy')
.Apply this diff:
copyCells: { text: langText['copy-cells'], - handler() { + async handler() { - this.onCopy('copy') + await this.onCopy('copy') },
Line range hint
113-115
: UpdatecutCells
handler to be asynchronousSimilarly, update the
cutCells
handler to beasync
and await the call tothis.onCopy('cut')
due to the asynchronous nature ofonCopy
.Apply this diff:
cutCells: { text: langText['cut-cells'], - handler() { + async handler() { - this.onCopy('cut') + await this.onCopy('cut') },
456-464
: Remove unnecessary call tosetCopyRange(virtualTable)
As the Clipboard API is now handling the copy operation directly, the call to
this.setCopyRange(virtualTable)
is no longer needed and can be removed.Apply this diff:
const virtualTable = this.createVirtualTable(selectedTds, operation) - this.setCopyRange(virtualTable) this.tableSelection.preSelectedTable = virtualTable this.tableSelection.preSelectedTds = selectedTds const blob = new Blob([this.tableSelection.preSelectedTable.outerHTML], { type: 'text/html' })
packages/fluent-editor/src/table/modules/table-operation-menu.ts
Outdated
Show resolved
Hide resolved
packages/fluent-editor/src/table/modules/table-operation-menu.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (1)
packages/fluent-editor/src/table/modules/table-operation-menu.ts (1)
Line range hint
475-479
: Improve error handling for cut operationThe cut operation should only proceed if the copy operation was successful.
Apply this diff:
async onCopy(operation) { const { selectedTds } = this.tableSelection const virtualTable = this.createVirtualTable(selectedTds, operation) this.setCopyRange(virtualTable) this.tableSelection.preSelectedTable = virtualTable this.tableSelection.preSelectedTds = selectedTds - await this.writeToClipboard(this.tableSelection.preSelectedTable.outerHTML) - if (operation === 'cut') { - const tableContainer = Quill.find(this.table) - tableContainer.emptyCells(selectedTds) + try { + await this.writeToClipboard(this.tableSelection.preSelectedTable.outerHTML) + if (operation === 'cut') { + const tableContainer = Quill.find(this.table) + tableContainer.emptyCells(selectedTds) + } + } catch (error) { + console.error('Copy/Cut operation failed:', error) + throw error } }
ca7b2c3
to
6d8ab96
Compare
a9ea4dc
to
8564dc5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@qwangry 这个issue的本意是把废弃的execCommand api移除,目前的代码依然是要兼容execCommand,这是没有必要的。
2847352
to
25dc3a5
Compare
@qwangry
这个有好的替代方案的吗? |
没搜到很好的方案,试了一下mousedown事件监听器,好像没用(⊙﹏⊙) |
我测试了下,好像新版本的火狐,并没有出现默认的拖拽大小的手柄了,所以这段代码可以移除。 |
不是要禁止调整大小吗,我测试的有这段代码也可以调整图片大小 |
这段代码的作用是禁用火狐浏览器自带的默认的调整图片大小的功能,确保只使用我们自定义的这个调整图片大小的样式。 如果不禁用火狐默认的调整图片大小的功能,在火狐下会出现两个调整图片大小的样式,不好看。 不过我尝试了下新版本的火狐,默认已经没有调整图片大小的功能了,所以这段代码实际上并没有作用。 |
@qwangry 我特意下载了一个老版本的火狐浏览器,给你看下这个效果 |
@qwangry 我也发现了,看来这段代码目前并不能禁用掉火狐这个默认调整大小的功能。 |
Fixed in #142 |
@all-contributors please add @qwangry for code. |
I've put up a pull request to add @qwangry! 🎉 |
|
PR
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: #97
What is the new behavior?
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit
Summary by CodeRabbit