GitHub sync #208

nshahan · 2018-01-09T18:53:29Z

ATTENTION: DO NOT MERGE THIS CHANGE WITHOUT UPDATING THE PUBSPEC
Waiting for sass_builder to publish an updated version.

Adds support for building with build_runner.
Sync recent changes for all components.

Can test new build process with https://github.com/dart-lang/angular_components_example/tree/support-build-runner

Otherwise element.scrollIntoView() could be called on the item (via ActiveItemMixin) while it was hidden or at (0, 0), causing the browser to scroll to the top of the page when the dropdown was opened. PiperOrigin-RevId: 179624999

PiperOrigin-RevId: 179704935

PiperOrigin-RevId: 179710605

Based on feedback from external sync #203 PiperOrigin-RevId: 179729861

PiperOrigin-RevId: 179735522

…Move the initialization of the component into ngChanges so that if the componentFactory, and the value both change in the same turn the component will be initialized with the correct value right away. PiperOrigin-RevId: 179756891

It took me about half an hour to figure this one out, so this short comment might save the next person some time. PiperOrigin-RevId: 179785103

PiperOrigin-RevId: 179819717

PiperOrigin-RevId: 179858020

…n auto select and dropdown select components. PiperOrigin-RevId: 179943921

PiperOrigin-RevId: 180303717

PiperOrigin-RevId: 180601492

PiperOrigin-RevId: 180768448

PiperOrigin-RevId: 180886341

This is to prepare for the new language restriction: dart-lang/sdk#30326 PiperOrigin-RevId: 180941194

PiperOrigin-RevId: 180971363

This matches the styling of disabled <material-checkbox>. Update disabled .label-description to match; remove manual styling that was making the text lighter. PiperOrigin-RevId: 180978929

Add exclusion for unused file. Update CHANGELOG.md, README.md with notes. Bump dependency versions. PiperOrigin-RevId: 181244068

googlebot · 2018-01-09T18:53:36Z

So there's good news and bad news.

👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there.

😕 The bad news is that it appears that one or more commits were 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 here in the pull request.

Note to project maintainer: This is a terminal state, meaning the cla/google commit status will not change from this State. It's up to you to confirm consent of the commit author(s) and merge this pull request when appropriate.

chalin · 2018-01-09T19:38:13Z

I'm looking at this now. Btw, I still can't generate the dartdocs for this version either. I.e., I get the same error as that reported in #207.

cc @kwalrath

chalin

LGTM. There is only one minor comment about repeated link definitions in markdown. Also, I haven't tried out the new build process (I'll do that a little later).

chalin · 2018-01-09T19:43:31Z

CHANGELOG.md

+is evolving. We expect to no longer require overrides once we are at a beta
+release, but this is unlikely until sometime in early 2018.
+
+[dep_overrides]: https://www.dartlang.org/tools/pub/dependencies#dependency-overrides


Minor: rather than repeat this link definition (which also appears on L19), it might be preferable to include it only once at the end of the file.

nshahan · 2018-01-16T17:03:25Z

Status: waiting for sass_builder update to be published. dart-league/sass_builder#17

This was for AngularDart v1. PiperOrigin-RevId: 181273259

This allows replacing this client template: <material-tab-strip [activeTabIndex]="activeTabIndex" (tabChange)="onTabChange($event)"> with this slightly more idiomatic client template: <material-tab-strip [(activeTabIndex)]="activeTabIndex"> PiperOrigin-RevId: 181340697

…enderer is used. PiperOrigin-RevId: 181342963

…n it closes. Current behavior: - If filter is inside popup, doesn't focus on open or close. - If filter is not inside popup, focuses on both open and close. PiperOrigin-RevId: 181345927

PiperOrigin-RevId: 181372356

PiperOrigin-RevId: 181381258

Using dynamic as bottom will become an error in Dart 2.0. dart-lang/sdk#29630 PiperOrigin-RevId: 182020806

Previously there was no visual indicator that the input was invalid when this mixin was used. PiperOrigin-RevId: 182085196

…s size. PiperOrigin-RevId: 182269449

Fixes an issue where ActiveItemMixin would cause the entire page to scroll unnecessarily in Firefox. PiperOrigin-RevId: 182272488

… case. Follow up CL updates the tools cell to use new interface PiperOrigin-RevId: 182289314

PiperOrigin-RevId: 182346419

In Dart 2.0, assert will only accept booleans, not closures any more. The solution to upgrade is very simple: we just self-execute any closures passed to assert today. PiperOrigin-RevId: 182406596

Remove unused table_selection_model.dart file. Cleanup outdated Copybara transformations. Bump sass_builder dependency. PiperOrigin-RevId: 182441504

nshahan · 2018-01-18T23:03:52Z

@chalin One more sync of changes in and now ready to publish.

Travis is failing because of a pub <--> sdk dev.18 issue at the moment but you can test again with the example branch https://github.com/dart-lang/angular_components_example/tree/support-build-runner.

chalin · 2018-01-18T23:05:59Z

Ok, will take a look first thing tomorrow!

cc @kwalrath

chalin · 2018-01-19T13:55:22Z

lib/utils/angular/properties/properties.dart

@@ -68,12 +51,39 @@ bool getBool(inputValue) {
      inputValue, 'inputValue', 'Expected a String, or bool type');
 }

+/// Parses html attribute [String] to a [bool].


Nit: html --> HTML

chalin · 2018-01-19T16:21:13Z

lib/utils/angular/properties/properties.dart

+///
+/// Should be used to parse values passed to @Attribute constructor argument.
+///
+/// This does not fully follow the HTML boolean attribute definition


This does not fully follow the HTML boolean attribute definition (https://stackoverflow.com/a/4139805)

"Does not fully follow" would mean that it supports a subset of the definition, but that in theory it could be extended/completed so that it does follow the full definition. This is not the case.

IMHO, this function is in conflict with the HTML boolean attribute definition, since it matches the HTML boolean attribute definition only for the case where the attribute value is an empty string, and it goes against the definition for other cases (where the value is "true" or "false", and it fails to recognize the case where the value matches the attribute name, ignoring case). In fact, this throws an error if the value matches the attribute name.

To make this clear, maybe this function should be named ngAttributeToBool() to be in contrast with what might be named htmlAttributeToBool().

cc @kwalrath @matanlurey

@rkj Any thoughts on rewording the comments here for more clarity? We can update them in a future sync to github.

chalin · 2018-01-19T16:22:16Z

lib/utils/angular/properties/properties.dart

+/// to false value.
+///
+/// When no attribute is present [defaultValue] value is returned (by default
+/// false).


Nit: a default of the default :) ... I would keep this DRY and drop the "(by default false)" since this can be reckoned by reading the function signature.

chalin · 2018-01-19T16:27:01Z

lib/utils/angular/properties/properties.dart

+  if (inputValue == null) return defaultValue;
+  return _parseBool(inputValue);
+}
+
 /// A typed version of [getDynamic].
 ///
 /// If [inputValue] is an [int], returns it.
 /// If [inputValue] is a `null`, returns [defaultValue].
 /// If [inputValue] is a String, parses using [onString], or uses [int.parse].


There is no use of onString() in the function body.

chalin · 2018-01-19T16:34:45Z

CHANGELOG.md

+   * Wait for the popup to be visible before activating dropdown items.
+   * Stop escape keyboard events from propagating after they are handled.
+   * Disallow deselection when clicked on a selected item with single selection
+   * models.


* models. --> models. since it is a continuation from the previous bullet.

chalin · 2018-01-19T16:35:59Z

CHANGELOG.md

+ * Material Tree: Autofocus the filter when `MaterialTreeDropdown` is opened,
+   but not when it closes.
+ * Better support for the zippy expansion case in selection model.
+ * Deprecate getBool, angular supports it now natively.


angular --> Angular

Btw, our convention in the docs is for method/function names to be followed by parentheses:

getBool --> getBool().

chalin · 2018-01-19T16:37:42Z

CHANGELOG.md

+ * Add a feature detector for `position: sticky`.
+ * Migrate from Glyph to Material Icon.
+ * Remove unused table model.
+ * Remove the getDynamic method from properties helpers.


getDynamic --> getDynamic()

chalin · 2018-01-19T16:37:51Z

CHANGELOG.md

+ * Remove uses of .runtimeType.
+ * Fix uses_bottom_as_dynamic ("fuzzy arrow") errors.
+ * Cleanup unneeded `preserveWhitespace: false` and `preserveWhitespace: true`.
+ * Cleanup type warnings and other Dart 2.0 fixes.


Dart 2.0 --> Dart 2

chalin · 2018-01-19T16:38:18Z

CHANGELOG.md

@@ -12,6 +72,12 @@ dependency_overrides:
  analyzer: ^0.31.0-alpha.1
 ```

+This is because Angular is starting to use and support the Dart 2.0.0 SDK, which


Dart 2.0.0 --> Dart 2

chalin · 2018-01-19T16:40:30Z

lib/utils/angular/properties/properties.dart

+  if (inputValue == null) return defaultValue;
+  return _parseBool(inputValue);
+}
+
 /// A typed version of [getDynamic].


getDynamic() has been removed from this file so the doc comment shouldn't refer to it.

chalin

A few nits, but I do have one more significant concern about the doc comments for attributeToBool(). None of it is gating, of course. Please see inlined comments.

chalin · 2018-01-19T16:46:55Z

@nshahan - FYI, our AngularDart example apps have been converted to use build_runner, and the apps using angular_components run just fine (e.g., as shown for the template-syntax example app in this Travis log).

PiperOrigin-RevId: 182548382

…rning. Tested: https://test.corp.google.com/ui#id=OCL:182564188:BASE:182564721:1516390579043:eb6c9c65 Failures are flakes or already failing. PiperOrigin-RevId: 182584854

chalin · 2018-01-20T13:29:09Z

CHANGELOG.md

+
+> NOTE: This code is considered production quality, but depends on angular:
+> 5.0.0-alpha+3. The alpha tag represents the evolving nature of the AngularDart
+> api, not code quality (5.0.0-alpha+3 is used in production Google apps).


The Angular version was bumped up (from +3 to +4) in the pubspec.yaml, just before merging. This changelog entry should be referring to 5.0.0-alpha+4.

Context: #208 (review) Not sure about renaming the method, I doubt we need the html version. PiperOrigin-RevId: 182801890

zacharydenton and others added 18 commits January 8, 2018 17:42

Make material-checkbox Focusable.

ed656d3

PiperOrigin-RevId: 179704935

Remove unused variable.

1742fc6

PiperOrigin-RevId: 179710605

Clarify file comment about angular dependency.

5100c4c

Based on feedback from external sync #203 PiperOrigin-RevId: 179729861

Fix typo in comment: expect --> except

83b7d28

PiperOrigin-RevId: 179735522

Clarify that icon names cannot have whitespace

965b37d

It took me about half an hour to figure this one out, so this short comment might save the next person some time. PiperOrigin-RevId: 179785103

Fix type for StringSelectionOptions.

db878b3

PiperOrigin-RevId: 179819717

Migrate material-tree from glyph to material-icon.

f34f494

PiperOrigin-RevId: 179858020

Stop escape keyboard events from propagating after they are handled i…

58327b0

…n auto select and dropdown select components. PiperOrigin-RevId: 179943921

Migrate scorecard to preserveWhitespace: false.

586e422

PiperOrigin-RevId: 180303717

Add custom builder for .scss.css files

a3aef0a

PiperOrigin-RevId: 180601492

Set Material Popup overscroll-behavior style property to 'contain'.

210a625

PiperOrigin-RevId: 180768448

Update material FAB icon shadow depth

ad53808

PiperOrigin-RevId: 180886341

Fix asserts to strictly accept a bool, not a closure.

8290364

This is to prepare for the new language restriction: dart-lang/sdk#30326 PiperOrigin-RevId: 180941194

Migrate material_yes_no_buttons to preserveWhitespace: false.

451d7af

PiperOrigin-RevId: 180971363

Use a darker color for disabled <material-radio> text.

2ce7edd

This matches the styling of disabled <material-checkbox>. Update disabled .label-description to match; remove manual styling that was making the text lighter. PiperOrigin-RevId: 180978929

Prep weekly sync to github.

27fde2f

Add exclusion for unused file. Update CHANGELOG.md, README.md with notes. Bump dependency versions. PiperOrigin-RevId: 181244068

nshahan requested a review from chalin January 9, 2018 18:53

googlebot added the cla: no label Jan 9, 2018

chalin approved these changes Jan 9, 2018

View reviewed changes

matanlurey and others added 6 commits January 18, 2018 13:47

Remove uses of //third_party/dart/di.

0f18e99

This was for AngularDart v1. PiperOrigin-RevId: 181273259

Fixes MaterialSelectItem displaying two times the label when factoryR…

16e56a6

…enderer is used. PiperOrigin-RevId: 181342963

Autofocus the filter when MaterialTreeDropdown is opened, but not whe…

23faeba

…n it closes. Current behavior: - If filter is inside popup, doesn't focus on open or close. - If filter is not inside popup, focuses on both open and close. PiperOrigin-RevId: 181345927

Add option to disable select all in the table selection model

a0369dc

PiperOrigin-RevId: 181372356

Deprecate getBool, angular supports it now natively.

aa17947

PiperOrigin-RevId: 181381258

srawlins and others added 8 commits January 18, 2018 14:01

Fix uses_bottom_as_dynamic ("fuzzy arrow") errors for material_select.

95f9637

Using dynamic as bottom will become an error in Dart 2.0. dart-lang/sdk#29630 PiperOrigin-RevId: 182020806

Don't override material-input underline color when the input is invalid.

7ef89c5

Previously there was no visual indicator that the input was invalid when this mixin was used. PiperOrigin-RevId: 182085196

Add ResizeObserver bindings + a Stream API for monitoring an Element'…

d7ca32e

…s size. PiperOrigin-RevId: 182269449

Use scrollIntoView({block: "nearest", inline: "nearest"}) if possible.

5321e03

Fixes an issue where ActiveItemMixin would cause the entire page to scroll unnecessarily in Firefox. PiperOrigin-RevId: 182272488

Updated: Modify selection model to better support the zippy expansion…

371aea1

… case. Follow up CL updates the tools cell to use new interface PiperOrigin-RevId: 182289314

Make it possible to customize menu button content.

5a8d0f3

PiperOrigin-RevId: 182346419

Self-execute all closures passed to assert.

322421f

In Dart 2.0, assert will only accept booleans, not closures any more. The solution to upgrade is very simple: we just self-execute any closures passed to assert today. PiperOrigin-RevId: 182406596

Prep to publish angular_components: 0.9.0-alpha+3.

c540000

Remove unused table_selection_model.dart file. Cleanup outdated Copybara transformations. Bump sass_builder dependency. PiperOrigin-RevId: 182441504

nshahan force-pushed the github-sync branch from 966efd2 to c540000 Compare January 18, 2018 22:56

chalin reviewed Jan 19, 2018

View reviewed changes

chalin approved these changes Jan 19, 2018

View reviewed changes

kevmoo and others added 2 commits January 19, 2018 14:03

Update dependencies

131aaf6

PiperOrigin-RevId: 182548382

Update loop to avoid a strong_mode_uses_dynamic_as_bottom analyzer wa…

5371573

…rning. Tested: https://test.corp.google.com/ui#id=OCL:182564188:BASE:182564721:1516390579043:eb6c9c65 Failures are flakes or already failing. PiperOrigin-RevId: 182584854

nshahan merged commit 204ffae into master Jan 19, 2018

nshahan deleted the github-sync branch January 19, 2018 22:36

chalin reviewed Jan 20, 2018

View reviewed changes

nshahan pushed a commit that referenced this pull request Feb 6, 2018

Remove misleading part of the doccomment.

9e379de

Context: #208 (review) Not sure about renaming the method, I doubt we need the html version. PiperOrigin-RevId: 182801890

nshahan pushed a commit that referenced this pull request Feb 7, 2018

Remove misleading part of the doccomment.

bf68e26

Context: #208 (review) Not sure about renaming the method, I doubt we need the html version. PiperOrigin-RevId: 182801890

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

GitHub sync #208

GitHub sync #208

nshahan commented Jan 9, 2018

googlebot commented Jan 9, 2018

chalin commented Jan 9, 2018

chalin left a comment

chalin Jan 9, 2018

nshahan commented Jan 16, 2018

nshahan commented Jan 18, 2018

chalin commented Jan 18, 2018

chalin Jan 19, 2018

chalin Jan 19, 2018 •

edited

Loading

nshahan Jan 19, 2018

chalin Jan 19, 2018

chalin Jan 19, 2018

chalin Jan 19, 2018

chalin Jan 19, 2018 •

edited

Loading

chalin Jan 19, 2018

chalin Jan 19, 2018

chalin Jan 19, 2018

chalin Jan 19, 2018

chalin left a comment

chalin commented Jan 19, 2018

chalin Jan 20, 2018

GitHub sync #208

GitHub sync #208

Conversation

nshahan commented Jan 9, 2018

googlebot commented Jan 9, 2018

chalin commented Jan 9, 2018

chalin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nshahan commented Jan 16, 2018

nshahan commented Jan 18, 2018

chalin commented Jan 18, 2018

Choose a reason for hiding this comment

chalin Jan 19, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chalin Jan 19, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chalin left a comment

Choose a reason for hiding this comment

chalin commented Jan 19, 2018

Choose a reason for hiding this comment

chalin Jan 19, 2018 •

edited

Loading

chalin Jan 19, 2018 •

edited

Loading