Skip to content

New widget icons and alignment added (#3195) #3215

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

Merged

Conversation

alexlindroos
Copy link
Contributor

Here is the PR for the new widget tree. Coloring of the widget tree and layout explorer is waiting for the new brand colors.

Before:
Screenshot 2021-07-23 at 16 08 39

After:

Screenshot 2021-07-23 at 15 56 45

@tomgilder tomgilder force-pushed the redesign/widget-tree-phase2 branch from 58123d6 to dab2be2 Compare July 29, 2021 20:28
@google-cla
Copy link

google-cla bot commented Jul 29, 2021

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

1 similar comment
@google-cla
Copy link

google-cla bot commented Jul 29, 2021

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

@tomgilder
Copy link
Contributor

@googlebot I consent

@tomgilder tomgilder force-pushed the redesign/widget-tree-phase2 branch from dab2be2 to 205b536 Compare July 29, 2021 21:08
@tomgilder
Copy link
Contributor

Sorry to push more changes to this PR, but I think it's better to review these changes all together.

The additional commit adds several more widget icons, and implements widget colors in the layout explorer. Because of showing the colors in layout explorer, the layout explorer now uses a thicker border and slight shadow to show the selected widget:

Screenshot_2021-07-29_at_23 59 40

@@ -0,0 +1,238 @@
import 'package:flutter/material.dart';
Copy link
Member

Choose a reason for hiding this comment

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

need copyright at top of file

),
);
);
});
Copy link
Member

Choose a reason for hiding this comment

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

nit: trailing comma

Copy link
Contributor

Choose a reason for hiding this comment

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

Fixed

return themeMap[_stripBrackets(widgetType)] ?? const WidgetTheme();
}

/// Strips the brackets of the widget
Copy link
Member

Choose a reason for hiding this comment

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

dartdoc nit. r/of/off and add new line after first sentence.

/// Strips the brackets off of the widget name.
///
/// For example: `AnimatedBuilder<String>` -> `AnimatedBuilder`.

Copy link
Contributor

Choose a reason for hiding this comment

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

Done

'Scrollbar': scrollTheme,
'ScrollConfiguration': scrollTheme,
'GridView': WidgetTheme(iconAsset: WidgetIcons.gridView),
'ListView': WidgetTheme(iconAsset: WidgetIcons.listView),
Copy link
Member

@kenzieschmoll kenzieschmoll Aug 2, 2021

Choose a reason for hiding this comment

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

put this WidgetTheme into a const above since it is used in multiple places. Here and elsewhere for all duplicated WidgetTheme instances

Copy link
Contributor

Choose a reason for hiding this comment

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

Done

Copy link
Member

@kenzieschmoll kenzieschmoll left a comment

Choose a reason for hiding this comment

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

LGTM after the last few comments are addressed

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.

3 participants