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

Update text selection when toggling checkbox #441

Merged
merged 3 commits into from
Jan 3, 2025

Conversation

amantoux
Copy link
Member

@amantoux amantoux commented Dec 19, 2024

When toggling a checkbox the cursor is set at the offset of the checkbox's line start

Fixes issues addressed by PR #400

@amantoux amantoux requested review from Amir-P and removed request for Amir-P December 19, 2024 11:13
@amantoux amantoux marked this pull request as draft December 21, 2024 11:28
@amantoux amantoux force-pushed the update_text_selection_when_toggling_checkbox branch from df73bcb to ce001ea Compare December 29, 2024 11:21
Copy link

codecov bot commented Dec 29, 2024

Codecov Report

Attention: Patch coverage is 92.85714% with 1 line in your changes missing coverage. Please review.

Project coverage is 88.24%. Comparing base (6abce16) to head (f987e9d).
Report is 3 commits behind head on master.

Files with missing lines Patch % Lines
.../fleather/lib/src/widgets/editable_text_block.dart 91.66% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##           master     #441   +/-   ##
=======================================
  Coverage   88.24%   88.24%           
=======================================
  Files          62       62           
  Lines       10630    10640   +10     
=======================================
+ Hits         9380     9389    +9     
- Misses       1250     1251    +1     

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

@amantoux amantoux force-pushed the update_text_selection_when_toggling_checkbox branch from b41905f to a044bb1 Compare December 29, 2024 11:24
@amantoux
Copy link
Member Author

@Amir-P can you please review?
Essentially, this allows the check box to render it's own state while silently updating the document via the controller.
This avoids the re-build of the editor that would scroll to the cursor position while preserving the current text selection.

@amantoux amantoux requested a review from Amir-P December 29, 2024 11:30
@amantoux amantoux marked this pull request as ready for review December 29, 2024 11:30
@amantoux amantoux force-pushed the update_text_selection_when_toggling_checkbox branch 2 times, most recently from c9fd3eb to ad0f7c3 Compare December 29, 2024 11:35
Copy link
Member

@Amir-P Amir-P left a comment

Choose a reason for hiding this comment

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

Looks like not notifying listeners is causing issues with undoing.

Steps to reproduce:

  1. Create a checklist
  2. Toggle it
  3. Press Cmd+Z
  4. Press Cmd +Z

Expected Behavior: After pressing Cmd +Z for the first time checklist should be unchecked and pressing Cmd +Z one more time reverts checklist creation.

Actual Behavior: After pressing Cmd +Z for the first time nothing happens and pressing Cmd +Z one more time reverts checklist creation.

Screen.Recording.2025-01-01.at.11.09.36.mov

@amantoux
Copy link
Member Author

amantoux commented Jan 1, 2025

Nice catch!
Can you please check the changes?

@amantoux amantoux force-pushed the update_text_selection_when_toggling_checkbox branch from ad0f7c3 to aad38c6 Compare January 1, 2025 12:39
@amantoux amantoux requested a review from Amir-P January 1, 2025 12:39
@amantoux amantoux force-pushed the update_text_selection_when_toggling_checkbox branch from aad38c6 to f987e9d Compare January 1, 2025 12:41
Copy link
Member

@Amir-P Amir-P left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks!

@amantoux amantoux merged commit 0240e29 into master Jan 3, 2025
4 checks passed
@amantoux amantoux deleted the update_text_selection_when_toggling_checkbox branch January 3, 2025 16:59
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