-
Notifications
You must be signed in to change notification settings - Fork 230
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
[SuperEditor][Mobile] Add caret customization (Resolves #1935) #2020
Conversation
@@ -177,6 +177,13 @@ class TestSuperEditorConfigurator { | |||
return this; | |||
} | |||
|
|||
/// Configures the [SuperEditor] to ignore the default layers and to use the given only |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like "the given" is repeated twice.
Also, this dart doc doesn't help me understand why this method exists, or when/why I might use it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated.
@@ -177,6 +177,16 @@ class TestSuperEditorConfigurator { | |||
return this; | |||
} | |||
|
|||
/// Configures the [SuperEditor] to use the given [documentLayers]. | |||
/// | |||
/// This can be used, for example, to customize the caret style for a specific platform. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems like a lot of responsibility to just change the caret style. Do we have similar examples of minor customizations that require this level of intervention?
I know we have keyboard handlers, but that situation is mostly about adding a few new handlers on top of existing ones.
In this case, the user is altering an existing UI property in a small way....
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated.
@@ -578,13 +603,20 @@ class _TestSuperEditorState extends State<_TestSuperEditor> { | |||
// iOS floating toolbar. | |||
const SuperEditorIosToolbarFocalPointDocumentLayerBuilder(), | |||
// Displays caret and drag handles, specifically for iOS. | |||
const SuperEditorIosHandlesDocumentLayerBuilder(), | |||
SuperEditorIosHandlesDocumentLayerBuilder( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What I was talking about in my layers comment was meant to go behind the specific place where custom layers were being created. I was also talking about how we expect app developers to customize these properties.
Am I reading this correctly that app developers will need to provide a custom list of layers to SuperEditor
if they want a 3px wide caret instead of a 2px wide caret?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Am I reading this correctly that app developers will need to provide a custom list of layers to SuperEditor if they want a 3px wide caret instead of a 2px wide caret?
Currently yes. We already have caret customization for desktop, which involves creating the whole list of layers...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
[SuperEditor][Mobile] Add caret customization. Resolves #1935
This PR adds the following customizations: