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: remove gap cursor #17888

Merged
merged 2 commits into from
Oct 10, 2023
Merged

fix: remove gap cursor #17888

merged 2 commits into from
Oct 10, 2023

Conversation

daibhin
Copy link
Contributor

@daibhin daibhin commented Oct 10, 2023

Problem

A lot of people don't understand what this is. Chatted about in https://posthog.slack.com/archives/C05N9R3HT7V/p1696434085225909

Changes

  • Remove it entirely

How did you test this code?

gap-insertion.mp4

@@ -65,6 +65,7 @@ export function Editor({
CustomDocument,
StarterKit.configure({
document: false,
gapcursor: false,
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this is mergeable. I find the weird gap cursor preferable to not being able to place a cursor between two nodes...

Not against this change, as long as we do the extra work to make sure you can add a line between nodes when we click there. Might be best done by detecting the selection and then inserting a line if there isn't one (not sure how hacky that would be)

@daibhin
Copy link
Contributor Author

daibhin commented Oct 10, 2023

@benjackwhite being honest I actually like gap cursors, but at least three people were confused by them. I used to reference Linear as an example of a tool that had a similar behaviour but when I went to reply in Slack it looks like they have removed theirs now too:

https://github.com/PostHog/posthog/assets/6685876/76e2b25b-28ca-4ad9-bf94-9e3a04f09f20
(No indication that the selection is between the two images after pressing the right arrow key or clicking between them until you type or press enter)

Maybe the industry is moving away from them as a concept 🤷 My personal vote is to leave the gap cursors alone

@benjackwhite
Copy link
Contributor

@benjackwhite being honest I actually like gap cursors, but at least three people were confused by them. I used to reference Linear as an example of a tool that had a similar behaviour but when I went to reply in Slack it looks like they have removed theirs now too:

https://github.com/PostHog/posthog/assets/6685876/76e2b25b-28ca-4ad9-bf94-9e3a04f09f20 (No indication that the selection is between the two images after pressing the right arrow key or clicking between them until you type or press enter)

Maybe the industry is moving away from them as a concept 🤷 My personal vote is to leave the gap cursors alone

@daibhin - my problem isn't with the cursor itself, but rather with the fact that this change actually removes the ability to place your cursor between two nodes 😅 Which is the bit I don't agree with. We should either fix that, or simply change this to hide the cursor when it is in a gap.

@daibhin daibhin requested a review from benjackwhite October 10, 2023 14:48
Copy link
Contributor

@benjackwhite benjackwhite left a comment

Choose a reason for hiding this comment

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

Now this is slick 👍

@daibhin daibhin merged commit 17b6409 into master Oct 10, 2023
74 checks passed
@daibhin daibhin deleted the dn-fix/hide-gap-cursor branch October 10, 2023 15:54
daibhin added a commit that referenced this pull request Oct 23, 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.

2 participants