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

Toolbar button styling to reflect cursor position when running on desktops with keyboard to move caret. #1801

Merged
merged 4 commits into from
Apr 13, 2024

Conversation

AtlasAutocode
Copy link
Collaborator

Description

Proposed changes make the toolbar buttons selection state reflect the styling as the cursor is moved. Using desktop (Windows) keyboard cursor keys to move the caret position (without selecting text).

  • Fix inline style tracking: When cursor is moved into a region of bold, the bold style is correctly recognized and reflected in the toolbar status. Problem: When the cursor keys move the caret out of the bold region, the bold style is not cleared and cannot be removed.

  • Fix: toggle_style_button failed to call default afterButtonPressed in base button settings. Constructor called options.afterButtonPressed rather than the class function afterButtonPressed that defaults to a call to QuillToolbarBaseButtonOptions.

  • Fix: quill_icon_button build when isSelected did not call afterButtonPressed (expected same action as when not selected).

  • Fix: QuillController _updateSelection removed param=source because not used; added new param insertNewline to recognize when a newline entered and maintain the correct inline style by correctly setting toggledStyle; updated replaceText to call _updateSelection for newline.

  • Fix: document.dart collectStyle: Click at the start of a line (of bold text), user would expect the style to be the visible style of the line including inline styles. Also requires updating insert.dart PreserveInlineStylesRule to recognize if inserting text at start of line and use attributes from the first char on line

  • Fix: color_button : Add afterPressed param to QuillToolbarIconButton

Related Issues

Improvements

Features

Additional notes

Suggestions

QuillController (public function) updateSelection also does not use the ChangeSource parameter.
ChangeSource enum does not appear to be used anywhere - is this deprecated or am I missing something?

Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I titled the PR using Conventional Commits.
  • I did not modify the CHANGELOG.md nor the plugin version in pubspec.yaml files.
  • All existing and new tests are passing.
  • I have run the commands in ./scripts/before_push.sh and it all passed successfully

Breaking Change

Does your PR require plugin users to manually update their apps to accommodate your change?

  • Yes, this is a breaking change (please indicate that with a ! in the title as explained in Conventional Commits).
  • No, this is not a breaking change.

Sorry, something went wrong.

Douglas Ward added 2 commits March 31, 2024 16:06

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
… call to class function afterButtonPressed to allow default call to base button settings

quill_icon_button: L26 build for isSelected updated to call afterButtonPressed = same as if not selected
QuillController _updateSelection removed param=source because not used; added new param insertNewline when true set tog to style of preceding char (last entered); updated replaceText to call _updateSelection for NL
document collectStyle:  Selecting the start of a line, user expects the style to be the visible style of the line including inline styles
insert at start of line uses style for line
@singerdmx
Copy link
Owner

Run dart format --set-exit-if-changed .
Formatted lib/src/widgets/quill/quill_controller.dart
Formatted 260 files (1 changed) in 1.33 seconds.
Error: Process completed with exit code 1.

@singerdmx
Copy link
Owner

@ellet0 I am unsure about this PR. Could you please take a look? Thanks!

@EchoEllet
Copy link
Collaborator

EchoEllet commented Apr 3, 2024

@ellet0 I am unsure about this PR. Could you please take a look? Thanks!

I'm not so sure about the changes in the insert rule and the document file

I haven't made any changes to the base of the library (in the document, raw editor widget, the rules, etc...)

Copy link
Collaborator

@EchoEllet EchoEllet left a comment

Choose a reason for hiding this comment

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

Can you revert all the unused comments like the ones in the example folder?

@AtlasAutocode
Copy link
Collaborator Author

Changes made and submitted.
This is my first contribution - do I need to do something??

@singerdmx
Copy link
Owner

@ellet0 can you check this PR again?

@singerdmx
Copy link
Owner

Run dart format --set-exit-if-changed .
Formatted lib/src/widgets/quill/quill_controller.dart
Formatted 261 files (1 changed) in 1.2[8](https://github.com/singerdmx/flutter-quill/actions/runs/8544426268/job/23524750239?pr=1801#step:13:9) seconds.
Error: Process completed with exit code 1.

Copy link
Owner

@singerdmx singerdmx left a comment

Choose a reason for hiding this comment

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

Please fix formatting issue

@singerdmx
Copy link
Owner

singerdmx commented Apr 8, 2024

We just need to test if it fixes the issue the PR mentioned.
If there is an issue, we can always roll back.

@singerdmx singerdmx merged commit caa5662 into singerdmx:master Apr 13, 2024
2 checks passed
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.

QuillToolbarBaseButtonOptions did'nt work flutter quill Bold not work
3 participants