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(edgeless): auto add frame on click when no frame exists #2905

Merged
merged 7 commits into from
May 31, 2023

Conversation

regischen
Copy link
Member

Closes #2846

@regischen regischen requested a review from doodlewind May 31, 2023 07:15
@codesandbox
Copy link

codesandbox bot commented May 31, 2023

This branch is running in CodeSandbox. Use the links below to review this PR faster.


CodeSandbox logoCodeSandbox logo  Open in CodeSandbox Web Editor | VS Code | VS Code Insiders | Preview

@vercel
Copy link

vercel bot commented May 31, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
blocksuite ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 31, 2023 9:44am
blocksuite-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 31, 2023 9:44am

@@ -36,6 +36,7 @@ import {
isEmbed,
isImage,
isInsidePageTitle,
isInTitleBlock,
Copy link
Member

Choose a reason for hiding this comment

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

Actually we don't have a title block, it's just the title field in the page block. And can we reuse the isInsidePageTitle method instead of adding a new one?

Copy link
Member Author

Choose a reason for hiding this comment

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

ok, I didn't notice there already has one method

@@ -194,6 +197,15 @@ export class DefaultSelectionManager extends AbstractSelectionManager<DefaultPag
);
}

private _checkHasAnyFrameBlock() {
Copy link
Member

Choose a reason for hiding this comment

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

Semantically, a checkSomething method should not perform mutations.

@@ -203,6 +203,10 @@ export class Page extends Space<FlatBlockMap> {
);
}

hasFrameBlock() {
Copy link
Member

Choose a reason for hiding this comment

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

We should avoid adding page level API that is hard-coded to some specific block types.

Copy link
Member Author

Choose a reason for hiding this comment

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

ok, so I move this method to selection manager and turn it to private

@@ -28,7 +28,7 @@ export async function type(page: Page, content: string, delay = 50) {
export async function withPressKey(
page: Page,
key: string,
fn: () => Promise<void>
fn: () => Promise<void> = () => new Promise(resolve => resolve())
Copy link
Member

Choose a reason for hiding this comment

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

This method is designed to run the handler, so if you don't want to pass in the callback, using pressBackspace will be semantically a better fit.

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah, I should have found there is pressBackspace method.

@doodlewind doodlewind changed the title fix(edgeless): click auto add frame when no frame fix(edgeless): auto add frame on click when no frame exists May 31, 2023
Copy link
Member

@doodlewind doodlewind left a comment

Choose a reason for hiding this comment

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

I think this is a better idea than our discussion result, good job!

@doodlewind doodlewind merged commit e8afc09 into master May 31, 2023
10 of 12 checks passed
@doodlewind doodlewind deleted the fix-no-frame-in-default-mode branch May 31, 2023 09:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Archived in project
Development

Successfully merging this pull request may close these issues.

Deleting the last/only object (in Edgeless) breaks page mode
2 participants