-
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] Allow custom view models to be aware of node selection (Resolves #1944) #2063
Conversation
@kasyrm Could you please try this PR to see if it fits your needs? |
@@ -168,6 +169,11 @@ class SingleColumnLayoutSelectionStyler extends SingleColumnLayoutStylePhase { | |||
..selection = selection | |||
..selectionColor = _selectionStyles.selectionColor; | |||
} | |||
if (viewModel is SelectionAwareViewModelMixin) { |
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.
To stress test this approach, can you refactor the other view models to use this mixin, too? If a solution is versatile enough for all 3rd party components then it should be versatile enough for our existing components.
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.
super_editor/lib/src/default_editor/selection_aware_viewmodel.dart
Outdated
Show resolved
Hide resolved
this.caret, | ||
required this.caretColor, | ||
}) : super(nodeId: nodeId, maxWidth: maxWidth, padding: padding); | ||
}) : super(nodeId: nodeId, maxWidth: maxWidth, padding: padding) { | ||
super.selection = selection; |
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.
Can you use super
in the parameter list instead of down here?
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.
We can for nodeId, maxWidth and padding, but not for selection and selectionColor. Since the mixin can't have a constructor we cannot use the super
keyword here.
Yes! Thank you :) |
@angelosilvestre if that is assumed, it probably shouldn't get. Imagine something like a table component, or anything else more complicated than a singe block, that component would need to use its own definition of selection. |
@kasyrm @matthew-carroll It depends on the node type. For Any node can override |
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] Allow custom view models to be aware of node selection. Resolves #1944
Currently, during the styling pass, we only apply the selection and selection color to our own viewmodels:
TextComponentViewModel
,ImageComponentViewModel
andHorizontalRuleComponentViewModel
.Because of that, app-level view models, don't receive this information
Screen.Recording.2024-05-30.at.11.33.50.mov
This PR introduces a
SelectionAwareViewModelMixin
. Any view model that mixes it in will receive theDocumentNodeSelection
and selection color during the style pass.Using this mixin, app-level components can easily render their selection:
Screen.Recording.2024-05-30.at.11.36.11.mov