-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Feature Request - Extract and wrap with a single child widget #52567
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
Comments
That's an interesting idea. I'm guessing that in the initial example that might look something like the following list:
I'm also guessing that we'd stop before any widget with multiple widgets, because the number of combinations could be large and the options could be confusing. |
What combinations? My understanding of the request is that we start at a child widget, and while keeping it where it is, extract a few layers of widgets around it. And if these layers reference other widgets, so let is be, these widgets will go into the extracted widget. And whatever variables these layers reference, will become formal constructor parameters of the new widget. |
I was assuming that the user would select the top-most widget to be extracted, and that we needed to identify the lowest child being included in the extraction. Starting at the bottom simplifies our task, but I don't know if that's how users would expect to use the feature. |
Personally I don't have any preference, so if starting at the bottom makes things easier, then I don't see any reason to oppose it. |
Maybe instead of supporting N layers of refactoring we suggest a few, like:
|
@johnpryan I don't understand what semantics you imply by saying "Extract 2 widgets and wrap with new widget". My understanding is that it would say "Extract 2 widgets as a new widget wrapping of the selected widget". Anyway, I think visually expanding selection is better, rather than to force users to count. |
It definitely would be (visual is almost always better than textual), but I don't know how we would highlight multiple widgets with a hole in the middle for the widget that we wouldn't be extracting. @DanTup Is that something we can do using LSP (both highlighting two disjoint regions and providing multiple ranges that users can move between)? If we can't show this visually then we'll need text that defines the range of widgets being extracted in a clear and concise way. That includes indicating whether the widgets are from the selection up the tree or from the selection down the tree. |
It seems to be possible in IntelliJ.
We need these popup items anyway, highlighting is just a nice addition to it. |
I'm not sure if I completely understand exactly the flow. We can't do anything while the user is moving up and down items in the CodeAction list (we have no code executing there). When are are running code, we could highlight code in VS Code by using decorations (this was the suggestion when I asked for an API for highlighting code), but it'd have to be done in the extension (there's no LSP standard for this). I wonder if an option for his would be to allow the user to just create a selection over the widgets to be extracted (before opening the lightbulb menu)? ![]() The CodeAction text could be written to indicate this in some way ("Extract SizedBox + ColouredBox", "Extract SizedBox (+ 4 widgets) + ColouredBox"). |
I don't think we know the flow yet; that's what we're exploring. IIRC, on the IntelliJ side, at least for 'Extract local variable' (I think that's what was being used), the flow is that the user selects the refactoring from the menu and then a menu pops up to allow the user to select the expression to be used as an initializer. Or in the case of this refactoring, possibly some description of which widgets are to be extracted. I'm guessing that on the VS Code side the equivalent would be to have the refactoring return a list of ranges, use the prompt box to present the list, change the highlight range as the user arrows through the list, and then apply the selected set of edits. On the IntelliJ side we have the ability to have a longer running dialog between the client and the server than what we're currently supporting in LSP, so we could delay computing the changes until we know which widgets are selected. But that's an implementation detail, not a UX design question. |
Yep, I think we could do that (although it would be VS Code specific - or at least, require some custom client code and LSP requests which in theory other editors could implement similar to the other new refactor work). |
Is your feature request related to a problem? Please describe.
Often I need to extract a bigger piece of widget tree to a widget that exposes a single
child
property. This may not be entire subtree but some part of it e.g. fromA->B->C->D
I want to get toA->B'->D
. To achieve that I create a new widget withchild
property and copy and paste part of the tree that needs to wrap this child. For example starting with following:I end up with this structure:
Describe the solution you'd like
I would like to have ability to wrap any widget with a single child wrapper widget similarly as I can extract part of subtree to a new widget.
Describe alternatives you've considered
Additional context
Copied from Dart-Code/Dart-Code#4566 as requested by @johnpryan
The text was updated successfully, but these errors were encountered: