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: Delete and backspace deleting block ids and breaking references #8974

Merged
merged 12 commits into from Apr 12, 2023

Conversation

megayu
Copy link
Collaborator

@megayu megayu commented Mar 30, 2023

Enhance delete and backspace behavior to avoid changing the UUID of a block if it contains refs.

Press delete at the end of a block, if there are no refs in the current block and it's not the last block of a page, then delete the current block and add the content of the current block to the start of the next block, otherwise delete the next block and add the content of next block to the end of the current block.

Press backspace at the start of a block, if there are refs in the current block and no refs in the prev block and they are belong to the same page, then delete prev block and add the content of prev block to the start of the current block, otherwise delete the current block and add the content of the current block to the end of the prev block.

Fix #8303

@tiensonqin tiensonqin requested a review from cnrpman April 3, 2023 07:20
@logseq-cldwalker logseq-cldwalker changed the title Enhance delete and backspace Fix: Delete and backspace deleting block ids and breaking references Apr 7, 2023
Copy link
Collaborator

@logseq-cldwalker logseq-cldwalker left a comment

Choose a reason for hiding this comment

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

@megayu Thanks for the fix with tests! 🐛 ⚡ This also fixes #8882. One weird edge case where we still break a block ref with delete/backspace is when there are two blocks with ids next to each other - https://www.loom.com/share/4dec797c81194f3dbb57dd70f6ce403a. I think the current behavior is reasonable but just wanted to note it. The content for that case:

        - ((64303d5a-bd52-45b5-b077-b93a1386d10d))
        - no ref
          id:: 64303d5a-bd52-45b5-b077-b93a1386d10d
        - has a ref
          id:: 64303d5d-81dc-448c-ab31-9e056d08425d
        - ((64303d5d-81dc-448c-ab31-9e056d08425d))

@@ -82,7 +82,93 @@ test('delete and backspace', async ({ page, block }) => {
await page.keyboard.press('Delete', { delay: 50 })
expect(await page.inputValue('textarea >> nth=0')).toBe('te')

// TODO: test delete & backspace across blocks
// delete & backspace across blocks
await block.enterNext()
Copy link
Collaborator

Choose a reason for hiding this comment

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

❤️


(declare save-block!)

(defn- block-has-no-ref?
[block-or-uuid]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Optional: It could be nice to keep this fn simpler and have it just take an entity id (eid) like db/pull or https://cljdoc.org/d/datascript/datascript/1.4.2/api/datascript.core#entity. This would mean passing in the db/entity arg

Copy link
Contributor

@tiensonqin tiensonqin left a comment

Choose a reason for hiding this comment

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

Worked as described! 🚢 👍

@tiensonqin tiensonqin merged commit c90e266 into logseq:master Apr 12, 2023
13 checks passed
@tiensonqin tiensonqin deleted the enhance/delete-backspace branch April 12, 2023 04:17
tiensonqin added a commit that referenced this pull request Apr 17, 2023
tiensonqin added a commit that referenced this pull request Apr 17, 2023
tiensonqin added a commit that referenced this pull request Apr 18, 2023
tiensonqin added a commit that referenced this pull request Apr 18, 2023
tiensonqin added a commit that referenced this pull request Apr 18, 2023
tiensonqin added a commit that referenced this pull request Apr 18, 2023
tiensonqin added a commit that referenced this pull request Apr 18, 2023
* fix: broken outliner structure when DELETE at the beginning

related to #8974

* fix: 'Delete' key deletes entire set of blocks

close #9128

---------

Co-authored-by: Mega Yu <yuhg2310@gmail.com>
tiensonqin added a commit that referenced this pull request Apr 19, 2023
tiensonqin added a commit that referenced this pull request Apr 19, 2023
tiensonqin added a commit that referenced this pull request Apr 19, 2023
bendyorke pushed a commit that referenced this pull request May 9, 2023
…8974)

* press delete at the end of a block, if no refs in current block,
delete current block instead of next

* enhance backspace behavior to avoid broke ref

* fix press backspace at the end of the block of the end of the page issue

* add e2e-test for delete and backspace across blocks

---------

Co-authored-by: Junyi Du <junyidu.cn@gmail.com>
Co-authored-by: Tienson Qin <tiensonqin@gmail.com>
Co-authored-by: Gabriel Horner <97210743+logseq-cldwalker@users.noreply.github.com>
bendyorke pushed a commit that referenced this pull request May 9, 2023
* fix: broken outliner structure when DELETE at the beginning

related to #8974

* fix: 'Delete' key deletes entire set of blocks

close #9128

---------

Co-authored-by: Mega Yu <yuhg2310@gmail.com>
bendyorke pushed a commit that referenced this pull request May 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Deleting the previous empty block will cause current block deleted
4 participants