Some fixes for Standalone Function portal #2004

exc3eed · 2017-11-08T05:47:22Z

workaround for error thrown after creating subscription "ExpressionChangedAfterItHasBeenCheckedError: Expression has changed after it was checked"
Name and Subscription field in Create app resized.
Creating subscription with empty display name is not allowed.

4.Creating app with empty name is not allowed.

5. Header is now on top of sidenav and dashboard

…wn after creating subscription ExpressionChangedAfterItHasBeenCheckedError: Expression has changed after it was checked. Previous\n Name and Subscription field in Create app resized\n Creating subscription with empty display name is not allowed\n Creating app with empty name is not allowed

dnfclas · 2017-11-08T05:47:28Z

@exc3eed,
Thanks for having already signed the Contribution License Agreement. Your agreement was validated by .NET Foundation. We will now review your pull request.
Thanks,
.NET Foundation Pull Request Bot #Resolved

ehamai · 2017-11-08T15:01:48Z

AzureFunctions.AngularClient/src/app/side-nav/side-nav.component.ts

@@ -404,6 +409,9 @@ export class SideNavComponent implements AfterViewInit {
    // so we need to make sure we're always overwriting them.  But if we simply
    // set the value to the same value twice, no change notification will happen.
    private _updateSubDisplayText(displayText: string) {
+        if (this._subscriptionOptionsUpdated) { // BUGBUG this is an workaround for https://github.com/Azure/azure-functions-ux/issues/2003


_subscriptionOptionsUpdated [](start = 17, length = 27)

This flag seems redundant to _subscriptionOptionsInitialized and it looks like you'd get the same behavior if you used that flag instead. #WontFix

_subscriptionOptionsUpdated will be true when subscriptionOptions Initialized more than or equal to two times. but _subscriptionOptionsInitialized will be true after one initialization So the behavior is not same here

In reply to: 149693692 [](ancestors = 149693692)

Ah yes you're right. Sorry I misread it.

In reply to: 149792444 [](ancestors = 149792444,149693692)

ehamai · 2017-11-08T15:04:46Z

        .subscribe(info => {

I just realized that we're subscribing to startupinfo twice. I'm not positive, but it's possible that the update issue you're seeing (#2003) has something to do with this. Instead, of subscribing to getStartupInfo again, just pass in the info object from the caller. That may resolve the ExpressionChangedAfterItHasBeenCheckedError you're seeing. #Resolved

Refers to: AzureFunctions.AngularClient/src/app/side-nav/side-nav.component.ts:428 in a153f32. [](commit_id = a153f32, deletion_comment = False)

ehamai

🕐

exc3eed · 2017-11-09T00:25:27Z

        .subscribe(info => {

Passing the info is a good idea but this is not resolving ExpressionChangedAfterItHasBeenCheckedError issue.

In reply to: 342844825 [](ancestors = 342844825)

Refers to: AzureFunctions.AngularClient/src/app/side-nav/side-nav.component.ts:428 in a153f32. [](commit_id = a153f32, deletion_comment = False)

ehamai · 2017-11-09T00:27:51Z

        .subscribe(info => {

Why won't it? Aren't you updating the list of subscriptions? Or am I misunderstanding the scenario where this is triggered? Can you just try it to see? I'd rather fix the root of the problem then create all these new properties for state management, especially when it's only used for Function runtime scenario's.

In reply to: 343007162 [](ancestors = 343007162,342844825)

Refers to: AzureFunctions.AngularClient/src/app/side-nav/side-nav.component.ts:428 in a153f32. [](commit_id = a153f32, deletion_comment = False)

…wn after creating subscription ExpressionChangedAfterItHasBeenCheckedError: Expression has changed after it was checked. Previous\n Name and Subscription field in Create app resized\n Creating subscription with empty display name is not allowed\n Creating app with empty name is not allowed

ahmelsayed · 2017-11-09T00:33:15Z

        .subscribe(info => {

regardless of it resolving the ExpressionChangedAfterItHasBeenCheckedError or not, you should not have an untracked subscription that doesn't get cleaned up before another is set up.

also if this PR is introducing ExpressionChangedAfterItHasBeenCheckedError, then it needs to be fixed in this PR

In reply to: 342844825 [](ancestors = 342844825)

Refers to: AzureFunctions.AngularClient/src/app/side-nav/side-nav.component.ts:428 in a153f32. [](commit_id = a153f32, deletion_comment = False)

ahmelsayed · 2017-11-09T00:34:22Z

AzureFunctions.AngularClient/src/app/side-nav/side-nav.component.ts

@@ -59,6 +61,7 @@ export class SideNavComponent implements AfterViewInit {
    public searchTerm = '';
    public hasValue = false;
    public tryFunctionApp: FunctionApp;
+    public headerOnTopOfSideNav =  this._scenarioService.checkScenario(ScenarioIds.headerOnTopOfSideNav).status === 'enabled';


move initialization to a constructor #Resolved

ahmelsayed · 2017-11-09T00:35:26Z

AzureFunctions.AngularClient/src/app/side-nav/side-nav.component.ts

-                let descriptor: SiteDescriptor;
+        const savedSubs = <StoredSubscriptions>this.localStorageService.getItem(LocalStorageKeys.savedSubsKey);
+        const savedSelectedSubscriptionIds = savedSubs ? savedSubs.subscriptions : [];
+        let descriptor: SiteDescriptor;


set type to SiteDescriptor | null #Resolved

exc3eed · 2017-11-09T00:39:47Z

        .subscribe(info => {

I filed a bug for it reproduction instruction is here https://github.com/Azure/azure-functions-ux/issues/2003

In reply to: 343007586 [](ancestors = 343007586,343007162,342844825)

Refers to: AzureFunctions.AngularClient/src/app/side-nav/side-nav.component.ts:428 in a153f32. [](commit_id = a153f32, deletion_comment = False)

ahmelsayed · 2017-11-09T00:42:47Z

AzureFunctions.AngularClient/src/app/side-nav/side-nav.component.ts

+        if (this._subscriptionOptionsInitialized) {
+            this._subscriptionOptionsUpdated = true;
+        }
+        this._subscriptionOptionsInitialized = true;


remove #Resolved

ahmelsayed · 2017-11-09T00:43:14Z

AzureFunctions.AngularClient/src/app/side-nav/side-nav.component.ts

@@ -68,6 +71,8 @@ export class SideNavComponent implements AfterViewInit {

    private _initialized = false;

+    private _subscriptionOptionsInitialized = false;
+    private _subscriptionOptionsUpdated = false;


remove #Resolved

ahmelsayed · 2017-11-09T00:43:57Z

AzureFunctions.AngularClient/src/app/side-nav/side-nav.component.ts

@@ -404,55 +407,59 @@ export class SideNavComponent implements AfterViewInit {
    // so we need to make sure we're always overwriting them.  But if we simply
    // set the value to the same value twice, no change notification will happen.
    private _updateSubDisplayText(displayText: string) {
+        if (this._subscriptionOptionsUpdated) { // BUGBUG this is an workaround for https://github.com/Azure/azure-functions-ux/issues/2003
+            return;
+        }


remove #Resolved

exc3eed · 2017-11-09T00:46:59Z

        .subscribe(info => {

What do you mean by untracked subscription? One way to fix the issue is using using ChangeDetectionStrategy.OnPush
angular/angular#6005

This seems right to me. But this is causing #1987
[Try Function] The Function App tree view is missing
as per Elliot

In reply to: 343008517 [](ancestors = 343008517,342844825)

Refers to: AzureFunctions.AngularClient/src/app/side-nav/side-nav.component.ts:428 in a153f32. [](commit_id = a153f32, deletion_comment = False)

ahmelsayed · 2017-11-09T01:02:32Z

        .subscribe(info => {

by untracked subscription I meant calling .subscribe() multiple times on a stream without properly disposing of the previous subscribe. Subscribe is not just a call back to 1 event, it'll run on every event from that source. If you setup a subscription that's not tracked to unsubscribe, then you are either creating a subscription that's expected to live for the life time of the app (e.g: see the getStartupInfo() subscription in the constructor. That's an untracked subscription. but that's okay because it's expected to have the life time of the app._ the other place where you may have an untracked subscription is if it's an http observable since though complete immediately. anything other than that, you need to save your subscription and figure out when to unsubscribe

In reply to: 343010787 [](ancestors = 343010787,343008517,342844825)

Refers to: AzureFunctions.AngularClient/src/app/side-nav/side-nav.component.ts:428 in a153f32. [](commit_id = a153f32, deletion_comment = False)

ahmelsayed · 2017-11-09T01:03:16Z

        .subscribe(info => {

read the reply from Drew Moore on that angular issue

In reply to: 343010787 [](ancestors = 343010787,343008517,342844825)

Refers to: AzureFunctions.AngularClient/src/app/side-nav/side-nav.component.ts:428 in a153f32. [](commit_id = a153f32, deletion_comment = False)

exc3eed · 2017-11-09T01:15:51Z

        .subscribe(info => {

I am lost. Can you suggest me what will be the right fix here?

In reply to: 343013546 [](ancestors = 343013546,343010787,343008517,342844825)

Refers to: AzureFunctions.AngularClient/src/app/side-nav/side-nav.component.ts:428 in a153f32. [](commit_id = a153f32, deletion_comment = False)

exc3eed · 2017-11-09T01:19:34Z

        .subscribe(info => {

It sounds like the issue is because displayText is updated internally from MultiDropdown list and also from onSubscriptionsSelect() method because subscriptionOption is updated. Maybe we shouldn't update displayText from two place

In reply to: 343015671 [](ancestors = 343015671,343013546,343010787,343008517,342844825)

Refers to: AzureFunctions.AngularClient/src/app/side-nav/side-nav.component.ts:428 in a153f32. [](commit_id = a153f32, deletion_comment = False)

exc3eed · 2017-11-09T01:23:41Z

        .subscribe(info => {

Ok just found a simplified workaround for it
setTimeout(() => {
this.subscriptionsDisplayText = '';
setTimeout(() => {
this.subscriptionsDisplayText = displayText;
}, 0);
});

In reply to: 343016367 [](ancestors = 343016367,343015671,343013546,343010787,343008517,342844825)

Refers to: AzureFunctions.AngularClient/src/app/side-nav/side-nav.component.ts:428 in a153f32. [](commit_id = a153f32, deletion_comment = False)

…wn after creating subscription ExpressionChangedAfterItHasBeenCheckedError: Expression has changed after it was checked. Previous\n Name and Subscription field in Create app resized\n Creating subscription with empty display name is not allowed\n Creating app with empty name is not allowed

ahmelsayed · 2017-11-09T17:28:45Z

AzureFunctions.AngularClient/src/app/side-nav/side-nav.component.scss

-
+.header-on-top-of-sidenav {
+    position: absolute;
+}


~~have you tested this in ibiza?~~ #WontFix

oh never mind, it's not an ibiza scenario #Resolved

this is not applicable for ibiza. it will be only applied for standalone function portal.

In reply to: 150031420 [](ancestors = 150031420)

ahmelsayed

Check one last time with @ehamai as well

ehamai · 2017-11-09T18:32:25Z

        .subscribe(info => {

I dunno, it seems pretty hacky to have 2 setTimeouts. Someone is definitely going to clean this up later and you'll be broken again. At the very least can you settle for one? BTW I still don't think that this addresses whatever the root cause of the problem is but I'm willing to compromise if you can just use one settimeout.

In reply to: 343017103 [](ancestors = 343017103,343016367,343015671,343013546,343010787,343008517,342844825)

Refers to: AzureFunctions.AngularClient/src/app/side-nav/side-nav.component.ts:428 in a153f32. [](commit_id = a153f32, deletion_comment = False)

exc3eed · 2017-11-09T20:54:44Z

        .subscribe(info => {

I have seen setTimeout is multiple places. It sounds like it is a workaround for component issue in this case MultidropdownList is also trying to update the disPlaytext.
Do we have a plan to fix component issue like this?
Seems like one settimeout is working. I am applying the fix

In reply to: 343248046 [](ancestors = 343248046,343017103,343016367,343015671,343013546,343010787,343008517,342844825)

Refers to: AzureFunctions.AngularClient/src/app/side-nav/side-nav.component.ts:428 in a153f32. [](commit_id = a153f32, deletion_comment = False)

…wn after creating subscription ExpressionChangedAfterItHasBeenCheckedError: Expression has changed after it was checked. Previous\n Name and Subscription field in Create app resized\n Creating subscription with empty display name is not allowed\n Creating app with empty name is not allowed

ehamai · 2017-11-09T20:56:25Z

        .subscribe(info => {

Yes and it's usually a hack to get around this problem. I'm saying that 2 setTimeouts really seems bad. Can you at least settle for one?

In reply to: 343288222 [](ancestors = 343288222,343248046,343017103,343016367,343015671,343013546,343010787,343008517,342844825)

Refers to: AzureFunctions.AngularClient/src/app/side-nav/side-nav.component.ts:428 in a153f32. [](commit_id = a153f32, deletion_comment = False)

ehamai · 2017-11-09T20:57:54Z

        .subscribe(info => {

Okay looking at your change, it looks like you used one setTimeout whereas your comment above had 2 which is why I was concerned.

In reply to: 343288687 [](ancestors = 343288687,343288222,343248046,343017103,343016367,343015671,343013546,343010787,343008517,342844825)

Refers to: AzureFunctions.AngularClient/src/app/side-nav/side-nav.component.ts:428 in a153f32. [](commit_id = a153f32, deletion_comment = False)

dnfclas added the cla-already-signed label Nov 8, 2017

exc3eed requested review from ehamai, shrishrirang, jvano and hartra344 November 8, 2017 05:47

exc3eed changed the title ~~Some fixes for Standalone Function portal\n workaround for error thro…~~ Some fixes for Standalone Function portal Nov 8, 2017

ehamai reviewed Nov 8, 2017

View reviewed changes

ehamai requested changes Nov 8, 2017

View reviewed changes

ahmelsayed reviewed Nov 9, 2017

View reviewed changes

ahmelsayed approved these changes Nov 9, 2017

View reviewed changes

exc3eed merged commit 2ff57eb into dev Nov 9, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Some fixes for Standalone Function portal #2004

Some fixes for Standalone Function portal #2004

exc3eed commented Nov 8, 2017 •

edited

dnfclas commented Nov 8, 2017 •

edited by exc3eed

ehamai Nov 8, 2017 •

edited by exc3eed

exc3eed Nov 8, 2017

ehamai Nov 8, 2017

ehamai commented Nov 8, 2017 •

edited by exc3eed

ehamai left a comment

exc3eed commented Nov 9, 2017 •

edited

ehamai commented Nov 9, 2017

ahmelsayed commented Nov 9, 2017

ahmelsayed Nov 9, 2017 •

edited by exc3eed

ahmelsayed Nov 9, 2017 •

edited by exc3eed

exc3eed commented Nov 9, 2017

ahmelsayed Nov 9, 2017 •

edited by exc3eed

ahmelsayed Nov 9, 2017 •

edited by exc3eed

ahmelsayed Nov 9, 2017 •

edited by exc3eed

exc3eed commented Nov 9, 2017

ahmelsayed commented Nov 9, 2017

ahmelsayed commented Nov 9, 2017

exc3eed commented Nov 9, 2017

exc3eed commented Nov 9, 2017 •

edited

exc3eed commented Nov 9, 2017

ahmelsayed Nov 9, 2017 •

edited by exc3eed

ahmelsayed Nov 9, 2017 •

edited by exc3eed

exc3eed Nov 9, 2017

ahmelsayed left a comment

ehamai commented Nov 9, 2017 •

edited

exc3eed commented Nov 9, 2017

ehamai commented Nov 9, 2017

ehamai commented Nov 9, 2017

Some fixes for Standalone Function portal #2004

Some fixes for Standalone Function portal #2004

Conversation

exc3eed commented Nov 8, 2017 • edited

dnfclas commented Nov 8, 2017 • edited by exc3eed

ehamai Nov 8, 2017 • edited by exc3eed

Choose a reason for hiding this comment

exc3eed Nov 8, 2017

Choose a reason for hiding this comment

ehamai Nov 8, 2017

Choose a reason for hiding this comment

ehamai commented Nov 8, 2017 • edited by exc3eed

ehamai left a comment

Choose a reason for hiding this comment

exc3eed commented Nov 9, 2017 • edited

ehamai commented Nov 9, 2017

ahmelsayed commented Nov 9, 2017

ahmelsayed Nov 9, 2017 • edited by exc3eed

Choose a reason for hiding this comment

ahmelsayed Nov 9, 2017 • edited by exc3eed

Choose a reason for hiding this comment

exc3eed commented Nov 9, 2017

ahmelsayed Nov 9, 2017 • edited by exc3eed

Choose a reason for hiding this comment

ahmelsayed Nov 9, 2017 • edited by exc3eed

Choose a reason for hiding this comment

ahmelsayed Nov 9, 2017 • edited by exc3eed

Choose a reason for hiding this comment

exc3eed commented Nov 9, 2017

ahmelsayed commented Nov 9, 2017

ahmelsayed commented Nov 9, 2017

exc3eed commented Nov 9, 2017

exc3eed commented Nov 9, 2017 • edited

exc3eed commented Nov 9, 2017

ahmelsayed Nov 9, 2017 • edited by exc3eed

Choose a reason for hiding this comment

ahmelsayed Nov 9, 2017 • edited by exc3eed

Choose a reason for hiding this comment

exc3eed Nov 9, 2017

Choose a reason for hiding this comment

ahmelsayed left a comment

Choose a reason for hiding this comment

ehamai commented Nov 9, 2017 • edited

exc3eed commented Nov 9, 2017

ehamai commented Nov 9, 2017

ehamai commented Nov 9, 2017

exc3eed commented Nov 8, 2017 •

edited

dnfclas commented Nov 8, 2017 •

edited by exc3eed

ehamai Nov 8, 2017 •

edited by exc3eed

ehamai commented Nov 8, 2017 •

edited by exc3eed

exc3eed commented Nov 9, 2017 •

edited

ahmelsayed Nov 9, 2017 •

edited by exc3eed

ahmelsayed Nov 9, 2017 •

edited by exc3eed

ahmelsayed Nov 9, 2017 •

edited by exc3eed

ahmelsayed Nov 9, 2017 •

edited by exc3eed

ahmelsayed Nov 9, 2017 •

edited by exc3eed

exc3eed commented Nov 9, 2017 •

edited

ahmelsayed Nov 9, 2017 •

edited by exc3eed

ahmelsayed Nov 9, 2017 •

edited by exc3eed

ehamai commented Nov 9, 2017 •

edited