Skip to content
This repository has been archived by the owner on Sep 5, 2024. It is now read-only.

fix(autocomplete): properly run animation for dialog in demo. #9437

Merged
merged 1 commit into from
Aug 31, 2016

Conversation

devversion
Copy link
Member

The autocomplete demo with the dialog currently logs a warning for the ng-cloak being present on the dialog template.

Removing ng-cloak removes that warning and also fixes the warning in the console.

@devversion devversion added the needs: review This PR is waiting on review from the team label Aug 28, 2016
@ThomasBurleson
Copy link
Contributor

ThomasBurleson commented Aug 28, 2016

@devversion - the ng-cloak was used to hide Layout Flexbox CSS changes that initial occur when the dialog is shown. Layout API has a ngCloak interceptor...

However, we should not hide the template... instead the developer can use ng-cloak on the content.

@devversion
Copy link
Member Author

devversion commented Aug 28, 2016

@ThomasBurleson - There is actually not explicit use of the Layout engine here, but anyways I added it as same as we did with the following commits.

I did not create an extra wrapper element for it, because it makes the demo source look more difficult than it needs to be..

@@ -9,7 +10,7 @@
</div>
</md-toolbar>

<md-dialog-content>
<md-dialog-content ng-cloak>
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the ng-cloak is only useful on the <md-dialog-content> wrapper. All others are superfluous.

Copy link
Member Author

@devversion devversion Aug 29, 2016

Choose a reason for hiding this comment

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

I can't understand why we even need ngCloak here - I will remove those though..

And just for undstanding, the dialog actions and toolbar are also components which can depend on the Flexbox Engine? Why do we need this on the content only?

Copy link
Contributor

Choose a reason for hiding this comment

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

The cloak hides the UI [at initialization] while more complex FlexBox layouts adjust and resize. The toolbar and actions are very simple and IMO do not require the hide trick. But if you want to be consistent, keep all three (3).

* The autocomplete demo with the dialog currently logs a warning for the `ng-cloak` being present on the dialog template.
  Removing `ng-cloak` removes that warning and also fixes the warning in the console.
@ThomasBurleson ThomasBurleson added pr: merge ready This PR is ready for a caretaker to review and removed needs: review This PR is waiting on review from the team labels Aug 29, 2016
@ThomasBurleson ThomasBurleson modified the milestone: 1.1.1 Aug 29, 2016
@hansl hansl merged commit 69607e0 into angular:master Aug 31, 2016
@devversion devversion deleted the fix/autocomplete-dialog-demo branch August 31, 2016 17:50
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
pr: merge ready This PR is ready for a caretaker to review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants