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: incorrect cursor movement behaviour #751

Merged
merged 6 commits into from
May 2, 2024

Conversation

Jei-sKappa
Copy link
Contributor

fixes #745
Previously the moveVertical function only moved the cursor 'node to node' without taking into consideration multiline nodes or some portion of a line with different fontSize (ex: a paragraph with size 16 but with a bolded word with size 32). This was caused by the PR #657 that closes #656 and #589.

@CLAassistant
Copy link

CLAassistant commented Mar 21, 2024

CLA assistant check
All committers have signed the CLA.

@LucasXu0
Copy link
Collaborator

LucasXu0 commented Mar 28, 2024

@Jei-sKappa Thanks for contributing. Please format the code.

@Jei-sKappa
Copy link
Contributor Author

@LucasXu0 Thanks for your reply! Based on the response of GitHub Action it seems like the problem is in the commit message and not in the code. Am I right?

@LucasXu0
Copy link
Collaborator

@LucasXu0 Thanks for your reply! Based on the response of GitHub Action it seems like the problem is in the commit message and not in the code. Am I right?

Except for the commit message, you need to format the Dart code too. Please click on the CI and check the error.

@Jei-sKappa Jei-sKappa force-pushed the fix_cursor_movement_745 branch from d7aac94 to 1c1c031 Compare March 28, 2024 09:48
Previously the `moveVertical` function only moved the cursor 'node to node' without thaking into consideration multiline nodes or some portion of a line with different fontSize (ex: a paragraph with size 16 but with a bolded word with size 32). This was caused by the PR AppFlowy-IO#657 that closes AppFlowy-IO#656 and AppFlowy-IO#589.
@Jei-sKappa Jei-sKappa force-pushed the fix_cursor_movement_745 branch from 1c1c031 to 7479f8c Compare April 13, 2024 15:30
@Jei-sKappa
Copy link
Contributor Author

@LucasXu0 Sorry for the late response.
I just pushed a new update to the pull requests that should fix the test's errors in the CI.
Unfortunately my first commit text exceeds the length defined in commitlint and the CI keeps failing 'cause of this.
Can I ignore it? If not what should I do to fix this?

Copy link

codecov bot commented Apr 16, 2024

Codecov Report

Attention: Patch coverage is 97.87234% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 74.86%. Comparing base (61d892a) to head (ebf0718).
Report is 8 commits behind head on main.

Files Patch % Lines
lib/src/extensions/position_extension.dart 97.87% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #751      +/-   ##
==========================================
- Coverage   75.29%   74.86%   -0.44%     
==========================================
  Files         300      300              
  Lines       14011    14178     +167     
==========================================
+ Hits        10550    10614      +64     
- Misses       3461     3564     +103     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Xazin
Copy link
Contributor

Xazin commented Apr 16, 2024

Can I ignore it? If not what should I do to fix this?

You don't have to worry about the commitlint failing, it's not important 👍

Copy link
Contributor

@Xazin Xazin left a comment

Choose a reason for hiding this comment

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

We need to introduce some more tests so we can stop a regression from occurring.

We can add tests in a new PR, I will open a new issue once @LucasXu0 has taken a look at this PR.

lib/src/extensions/position_extension.dart Outdated Show resolved Hide resolved
lib/src/extensions/position_extension.dart Outdated Show resolved Hide resolved
lib/src/extensions/position_extension.dart Outdated Show resolved Hide resolved
lib/src/extensions/position_extension.dart Outdated Show resolved Hide resolved
lib/src/extensions/position_extension.dart Outdated Show resolved Hide resolved
@Xazin Xazin requested a review from LucasXu0 April 30, 2024 20:39
@LucasXu0 LucasXu0 merged commit ffe4b08 into AppFlowy-IO:main May 2, 2024
10 of 12 checks passed
@Jei-sKappa Jei-sKappa deleted the fix_cursor_movement_745 branch May 2, 2024 07:09
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.

[Bug] Wrong cursor up/down movement behaviour [Bug] Heading level specific padding stops cursor
4 participants