-
-
Notifications
You must be signed in to change notification settings - Fork 346
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: selecting multiple blocks and dragging will show multiple placeholders #7089
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Ignored Deployment
|
Your org has enabled the Graphite merge queue for merging into masterAdd the label “merge” to the PR and Graphite will automatically add it to the merge queue when it’s ready to merge. You must have a Graphite account and log in to Graphite in order to use the merge queue. Sign up using this link. |
This stack of pull requests is managed by Graphite. Learn more about stacking. |
72cc081
to
f5d8743
Compare
@@ -103,11 +107,15 @@ export class ParagraphBlockComponent extends BlockElement< | |||
) | |||
return; | |||
|
|||
const selection = this.host.selection.value[0]; |
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.
Better to use selection.find('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.
Thanks for your comment! Code updated.
f5d8743
to
c460ef3
Compare
@@ -103,11 +107,15 @@ export class ParagraphBlockComponent extends BlockElement< | |||
) | |||
return; | |||
|
|||
const selection = this.host.selection.find('text'); | |||
const isCollapsed = | |||
selection instanceof TextSelection ? selection.isCollapsed : false; |
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.
Since you use find
method, you won't need to check the type of it. Also, isCollapsed
is a method, not a property.
So it can be replaced by selection?.isCollapsed() ?? false
.
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.
Got it! I will update this change in another PR, thanks for your advice.
Merge activity
|
…olders (#7089) ### What changed? - the placeholder will be visible, only if the selection is text selection and collapsed - add e2e test Fix affine-design/issue/BS-312. | Before | After | | ---- | ---- | | ![截屏2024-05-17 19.02.32.png](https://graphite-user-uploaded-assets-prod.s3.amazonaws.com/sJGviKxfE3Ap685cl5bj/54123270-fe12-4004-b99b-80d87196d106.png) | ![截屏2024-05-17 19.04.14.png](https://graphite-user-uploaded-assets-prod.s3.amazonaws.com/sJGviKxfE3Ap685cl5bj/b09fbb99-9a5b-4d1d-9637-3175a5c21efc.png)| ### How to test? Test the paragraph block in various states like editing, selecting, etc. Ensure the placeholder visibility is as expected. ---
ee14022
to
2a4f809
Compare
What changed?
Fix affine-design/issue/BS-312.
How to test?
Test the paragraph block in various states like editing, selecting, etc. Ensure the placeholder visibility is as expected.