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

Extract Widget: Update Constructor for Widget #2166

Closed
brianegan opened this issue Apr 27, 2018 · 9 comments
Closed

Extract Widget: Update Constructor for Widget #2166

brianegan opened this issue Apr 27, 2018 · 9 comments

Comments

@brianegan
Copy link
Contributor

brianegan commented Apr 27, 2018

Hey hey -- Another bit of feedback regarding extract widget! Thanks so much for your help and providing the functionality, really cool to use it :)

I'd love it if the new "Extract Widget" function created a constructor that follows the normal conventions for Widget constructors:

"By convention, widget constructors only use named arguments. Named arguments can be marked as required using @required. Also by convention, the first argument is key, and the last argument is child, children, or the equivalent."

From https://docs.flutter.io/flutter/widgets/StatefulWidget-class.html

Steps to Reproduce

  1. Select Widget
  2. Extract Widget
  3. See this constructor used: NewWidget(this.todo, this.taskKey, this.isEditing, this.noteKey);
  4. Would prefer to see this constructor used: NewWidget({Key key, this.todo, this.taskKey, this.isEditing, this.noteKey}) : super(key: key);

No need to add the @required fields, as that's more of a judgement call than an automatic refactoring.

Version info

[✓] Flutter (Channel beta, v0.3.1, on Mac OS X 10.13.4 17E199, locale en-US)
    • Flutter version 0.3.1 at /Users/phillywiggins/lab/flutter
    • Framework revision 12bbaba9ae (7 days ago), 2018-04-19 23:36:15 -0700
    • Engine revision 09d05a3891
    • Dart version 2.0.0-dev.48.0.flutter-fe606f890b

[✓] Android toolchain - develop for Android devices (Android SDK 27.0.3)
    • Android SDK at /Users/phillywiggins/Library/Android/sdk
    • Android NDK at /Users/phillywiggins/Library/Android/sdk/ndk-bundle
    • Platform android-27, build-tools 27.0.3
    • ANDROID_HOME = /Users/phillywiggins/Library/Android/sdk
    • Java binary at: /Applications/Android Studio.app/Contents/jre/jdk/Contents/Home/bin/java
    • Java version OpenJDK Runtime Environment (build 1.8.0_152-release-1024-b01)
    • All Android licenses accepted.

[✓] iOS toolchain - develop for iOS devices (Xcode 9.3)
    • Xcode at /Applications/Xcode.app/Contents/Developer
    • Xcode 9.3, Build version 9E145
    • ios-deploy 1.9.2
    • CocoaPods version 1.3.1

[✓] Android Studio (version 3.1)
    • Android Studio at /Applications/Android Studio.app/Contents
    • Flutter plugin version 23.2.2
    • Dart plugin version 173.4700
    • Java version OpenJDK Runtime Environment (build 1.8.0_152-release-1024-b01)

[✓] IntelliJ IDEA Community Edition (version 2018.1.2)
    • IntelliJ at /Applications/IntelliJ IDEA CE.app
    • Flutter plugin version 24.0.2
    • Dart plugin version 181.4668.60

[✓] VS Code (version 1.22.1)
    • VS Code at /Applications/Visual Studio Code.app/Contents
    • Dart Code extension version 2.11.2

[✓] Connected devices (1 available)
    • Android SDK built for x86 • emulator-5554 • android-x86 • Android 8.1.0 (API 27) (emulator)

• No issues found!

@pq
Copy link
Contributor

pq commented Apr 29, 2018

/cc @scheglov

@pq pq modified the milestones: On Deck, Backlog Apr 29, 2018
@scheglov scheglov self-assigned this Apr 29, 2018
@scheglov
Copy link
Contributor

But in the case when the refactoring creates parameters, these parameters not just @required, they are just required parameters and they are added because the code that is being extracted uses these parameters. Not providing them would make them null. Is it usually OK?

@scheglov
Copy link
Contributor

https://dart-review.googlesource.com/c/sdk/+/53060

dart-bot pushed a commit to dart-lang/sdk that referenced this issue Apr 30, 2018
…h 'key'.

R=devoncarew@google.com, paulberry@google.com

Bug: flutter/flutter-intellij#2166
Change-Id: Ie698dc27a1328b7e18e92f46583f84c22c84de57
Reviewed-on: https://dart-review.googlesource.com/53060
Reviewed-by: Paul Berry <paulberry@google.com>
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
@brianegan
Copy link
Contributor Author

brianegan commented Apr 30, 2018

@scheglov Fair point, might be best to mark em required since they are required in the initial extraction (not sure what ya did in that PR). As a heads up: You might need to ensure the document imports the foundation or meta packages in that case as well, since those modules define the @required attribute.

Thanks so much for the quick response!

@scheglov
Copy link
Contributor

scheglov commented May 7, 2018

More improvements for Extract Widget - the widgets we are extracting are StatelessWidget and their fields are final, so the constructor can be const. Also make parameters @required.

https://dart-review.googlesource.com/c/sdk/+/54066

dart-bot pushed a commit to dart-lang/sdk that referenced this issue May 7, 2018
…required named parameters.

R=brianwilkerson@google.com, pquitslund@google.com

Bug: flutter/flutter-intellij#2166
Change-Id: I28f548c8e814ac5b9585dd46fbb9a8f84f70f418
Reviewed-on: https://dart-review.googlesource.com/54066
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Phil Quitslund <pquitslund@google.com>
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
@lharland
Copy link

lharland commented Jun 2, 2018

How can I extract a widget? In both vscode and Android Studio.

@scheglov
Copy link
Contributor

scheglov commented Jun 2, 2018

Sorry, I don't use VS Code, cannot help you with this.

In IntelliJ base products (I use IDEA CE, but Android Studio should be the same) it is in Refactor | Extract | Flutter Widget. I gave it a keyboard shortcut for convenience, you can configure it in preferences.
image

image

@pq
Copy link
Contributor

pq commented Jun 2, 2018

/cc @DanTup

@DanTup
Copy link
Contributor

DanTup commented Jun 4, 2018

@leslieharland in VS Code the refactors can be found by clicking on the Light Bulb icon, pressing Ctrl+. (or on macOS, Cmd+.) or clicking Refactor in the context menu.

screen shot 2018-06-04 at 11 56 47 am

screen shot 2018-06-04 at 11 57 03 am

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants