-
-
Notifications
You must be signed in to change notification settings - Fork 415
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
feat(edgeless): add surface ref block #5013
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Ignored Deployment
|
I just updated the design doc. Feel free to post any comments. @doodlewind |
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.
The role of the
affine:surface-ref
is set to be hub, cause the content block cannot be the children of the root block. But the hub seems a little inappropriate.
@Saul-Mirone do you think we can break the previous assumption? In this case, affine:page
could have both affine:note
or affine:surface-ref
as its direct children.
match: element => element.model.role === 'content', | ||
match: element => | ||
element.model.role === 'content' || | ||
element.model.flavour === 'affine:surface-ref', |
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.
I think it's anti-pattern to hard-code affine
related logic in the lit package. @Saul-Mirone any advice?
@@ -145,10 +145,14 @@ export class BlockElement< | |||
return result; | |||
} | |||
|
|||
calculatePath = () => { |
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.
Why you need to expose this method?
match: element => element.model.role === 'content', | ||
match: element => | ||
element.model.role === 'content' || | ||
element.model.flavour === 'affine:surface-ref', |
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.
We shouldn't sync this block for native selection. Consider handle the edge case in Delete
keymap.
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.
We shouldn't hard code logic in lit and std package.
It's okay to refactor later. |
FIXME comment has been added. |
Thanks for the feat, looking forward to the upcoming housekeeping updates! |
I will add some tests later on. |
Close #5010
Change
affine:surface-ref
to hold edgeless content in doc modeDesign
The
affine:surface-ref
block is designed to be the children of theaffine:note
block. Thereference
property is the id of the surface element/children that are expected to be displayed in doc mode.In practice, any edgeless content can be displayed in doc mode through this block including shape, canvas text, brush, etc.
Regarding the rendering implementation, the
affine:surface-ref
block holds a surface renderer to render surface elements and creates a portal-like renderer for theaffine:note
block to render notes inside a frame.It will interpret the reference property and set the viewport according to the referenced element's
XYWH
property every time the frame changes.There are still some things need to be discuss:
affine:surface-ref
is set to behub
, cause thecontent
block cannot be the children of theroot
block. But thehub
seems a little inappropriate.Todo
There are some remains:
surface-ref
block can't be dragged using the drag-handler so far, it is necessary to consult with the designers.