-
-
Notifications
You must be signed in to change notification settings - Fork 2.9k
Copy-Paste Formatting Inheritance #7512
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
base: main
Are you sure you want to change the base?
Conversation
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.
This is great, thank you! Just one question on the behavior difference between signal and external text.
|
|
||
| // Check if we're selecting all content | ||
| const totalLength = this.quill.getLength(); | ||
| const isSelectingAll = selection.length >= totalLength - 1; |
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.
Ah, I see that quill.getLength() always includes an additional \n character, nice find!
ts/quill/signal-clipboard/index.ts
Outdated
| if (selection.length === 0) { | ||
| formats = this.quill.getFormat(selection.index); | ||
| } else if (isSelectingAll || !signal) { | ||
| // No formatting for select-all or external plain text |
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.
Not including formats for external text is a departure from current behavior, and I think means:
- select formatted section in the compose box (but not the whole selection)
- paste behavior of unformatted text now varies:
- paste with unformatted signal text -> inherits format
- paste with unformatted external text -> stays unformatted
I'm not sure we want that difference in behavior. Let's preserve the original behavior here, unless there's a reason for the change I'm not thinking of.
| import { SignalClipboard } from '../../quill/signal-clipboard/index.js'; | ||
| import { QuillFormattingStyle } from '../../quill/formatting/menu.js'; | ||
|
|
||
| class MockQuill { |
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.
this is very cool and will be useful!
|
Thank you for the feedback! I've updated index.dom.ts to not remove formatting when pasting external tests, as well as changed file names as needed. |
|
Thanks @brianHarder! This has been merged internally and will be included in the next beta release. |
Contributor checklist:
mainbranchpnpm run readyrun passes successfully (more about tests here)Description
This PR ensures that existing formatting is not inherited when unformatted text is pasted in to replace the existing text. To implement this change, index.ts in the ts/quill/signal-clipboard directory has been modified to add a check for select all. If a paste event happens when all text is selected, no formatting is applied. This PR partially addresses GH issue #6793.
I manually tested this PR by performing paste events in the mock environment. I tested that selecting all and pasting cleared the formatting and that pasting into a partial section kept the formatting. I also added a test file to simulate the paste all event in two scenarios - strikethrough and text with multiple formats applied - and check that no formatting is applied. I did all my testing on my laptop, which runs macOS 10.15.7.
Screen.Recording.2025-10-01.at.3.33.03.PM.mov