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

Arrow up selection fix #4536 #4544

Merged
merged 1 commit into from
May 25, 2023
Merged

Arrow up selection fix #4536 #4544

merged 1 commit into from
May 25, 2023

Conversation

fantactuka
Copy link
Contributor

That condition looks a bit odd and is likely redundant or missing extra logic. Basically any shift + arrow-up selection will be broken if going into { canBeEmpty: false } (tables, lists). Original issue uses image as an example, but even list + paragraph case is broken:

  1. Add list + paragraph, press shift + up in attempt to select
  2. Note that cursor just moves to the list, without selecting anything

Before:

Screen.Recording.2023-05-24.at.11.38.08.AM.mov

After:

Screen.Recording.2023-05-24.at.11.39.26.AM.mov

@vercel
Copy link

vercel bot commented May 24, 2023

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

Name Status Preview Comments Updated (UTC)
lexical ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 24, 2023 8:23pm
lexical-playground ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 24, 2023 8:23pm

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label May 24, 2023
@github-actions
Copy link

github-actions bot commented May 24, 2023

size-limit report 📦

Path Size Loading time (3g) Running time (snapdragon) Total time
packages/lexical/dist/Lexical.js 27.78 KB (0%) 556 ms (0%) 492 ms (+15.64% 🔺) 1.1 s
packages/lexical-rich-text/dist/LexicalRichText.js 38.71 KB (0%) 775 ms (0%) 504 ms (+14.61% 🔺) 1.3 s
packages/lexical-plain-text/dist/LexicalPlainText.js 38.69 KB (0%) 774 ms (0%) 258 ms (-58.47% 🔽) 1.1 s

@@ -375,7 +374,6 @@ test.describe('HTML Lists CopyAndPaste', () => {
await page.keyboard.press('Enter');
await page.keyboard.press('Enter');
await page.keyboard.press('ArrowUp');
await moveLeft(page, 4);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This test was wrong as moving upward was broken

@@ -13,6 +13,7 @@
position: relative;
outline: 0;
padding: 8px 28px 40px;
min-height: 150px;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Minor adjustments for mobile screen on a playground, not really a separate PR-worthy

@fantactuka fantactuka merged commit 413e8ed into main May 25, 2023
@zurfyx zurfyx mentioned this pull request May 26, 2023
@fantactuka fantactuka deleted the arrow-up-selection branch July 6, 2023 20:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants