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

p-dialog: ExpressionChangedAfterItHasBeenCheckedError with attr.aria-labelledby #13636

Closed
psarno opened this issue Sep 8, 2023 · 12 comments · Fixed by #13644
Closed

p-dialog: ExpressionChangedAfterItHasBeenCheckedError with attr.aria-labelledby #13636

psarno opened this issue Sep 8, 2023 · 12 comments · Fixed by #13644
Milestone

Comments

@psarno
Copy link

psarno commented Sep 8, 2023

Describe the bug

I know some work is being done here on accessability, and I saw a related issue but not with the p-dialog component.

We are on the latest PrimeNG version (16.3.1).

The exception is:

ExpressionChangedAfterItHasBeenCheckedError: Expression has changed after it was checked. Previous value for 'attr.aria-labelledby': 'pn_id_70_header'. Current value: 'pn_id_73_header'. Expression location: _Dialog component

Call Stack:

at throwErrorIfNoChangesMode (core.mjs:11010:11)
at bindingUpdated (core.mjs:14251:17)
at ɵɵattribute (core.mjs:14295:9)
at Dialog_div_0_div_1_Template (primeng-dialog.mjs:357:8)
at ReactiveLViewConsumer.runInContext (core.mjs:11103:13)

Line in primeng-dialog.mjs triggering this:

image

Dialog defined as:

<p-dialog
  [(visible)]="showUpdateMultiPointerDialog"
  [modal]="true"
>

Also - Can we get an updated reproducer template that isn't on Angular 14 and PrimeNG 14?

I am unable to update the template in stackblitz to Angular 16/PrimeNG 16, no matter what I try I end up with:

Error in /turbo_modules/primeng@16.3.1/fesm2022/primeng-tieredmenu.mjs (137:252)
Cannot access 'TieredMenu' before initialization

Environment

Angular 16.2.2
PrimeNG 16.3.1

Reproducer

No response

Angular version

16.2.2

PrimeNG version

16.3.1

Build / Runtime

Angular CLI App

Language

TypeScript

Node version (for AoT issues node --version)

18.16.0

Browser(s)

No response

Steps to reproduce the behavior

  1. Create a p-dialog component
  2. Set it's [(visible)] property to a boolean flag
  3. Set the boolean to true to show the dialog

Expected behavior

There is no ExpressionChangedAfterItHasBeenCheckedError

@psarno psarno added the Status: Needs Triage Issue will be reviewed by Core Team and a relevant label will be added as soon as possible label Sep 8, 2023
@SoyDiego
Copy link
Contributor

SoyDiego commented Sep 8, 2023

I think I did a PR about that.
I will check again.

Edit ⚠️
I found it.
#13538.
But now I have a doubt, your problem is in Panel o Dialog? Because you said Panel in your message but Dialog in the title and share a Stackblitz please

Thanks

@psarno psarno changed the title p-dialog: ExpressionChangedAfterItHasBeenCheckedError with attr.aria-labelledby p-panel: ExpressionChangedAfterItHasBeenCheckedError with attr.aria-labelledby Sep 8, 2023
@psarno psarno changed the title p-panel: ExpressionChangedAfterItHasBeenCheckedError with attr.aria-labelledby p-dialog: ExpressionChangedAfterItHasBeenCheckedError with attr.aria-labelledby Sep 8, 2023
@psarno
Copy link
Author

psarno commented Sep 8, 2023

@SoyDiego I appreciate the help.

It's in the dialog component, as per the stack trace:

at Dialog_div_0_div_1_Template (primeng-dialog.mjs:357:8)

And error:

Expression` location: _Dialog component

I see the PR was merged. Did that go out with 16.3.1?

If so then an issue persists here.

I am unable to do a Stackblitz, see the bottom of my post, there is a link there ... can't get it to work with 16.

@SoyDiego
Copy link
Contributor

SoyDiego commented Sep 8, 2023

Yes, was merged. It's very weird.
Tomorrow I will check again and I will update here.

@SoyDiego
Copy link
Contributor

SoyDiego commented Sep 8, 2023

Now I saw the problem.
My PR fixed in dynamic dialog and this is dialog. They are different. I will try to solve again tomorrow.

Thanks for report the issue

@SoyDiego
Copy link
Contributor

SoyDiego commented Sep 9, 2023

Hi @psarno I tried to replicate your problem but I cannot see errors and anything.

My code

image

image

Testing

testing dialog.

For that maybe is good idea if you can replicate with Stackblitz.

Thanks again

@psarno
Copy link
Author

psarno commented Sep 9, 2023

@SoyDiego It seems to be the exact same issue as was occuring in #13497 just in a different component.

It may be difficult to reproduce, I am not sure what the underlying cause is but it's probably no coincidence that we have the same error in two similar components on the same exact statement (with the attr.aria-labelledby).

Sometimes these things are not always immediately reproducible and we have to go off crash reports and put in defensive coding or fixes simply based off reports that include the call stack, such as this one does.

Since this looks to be literally the same problem, with the same code, just in a different component, why not apply the same fix here to be safe? It was needed in one, why would it not be needed in both?

image

And as I asked in the original post, is there any reason we don't have an updated Stackblitz repro template that uses the latest PrimeNG 16? I can not get 16 to work on StackBlitz, I am unsure why it throws this error.

@SoyDiego
Copy link
Contributor

SoyDiego commented Sep 9, 2023

@SoyDiego It seems to be the exact same issue as was occuring in #13497 just in a different component.

It may be difficult to reproduce, I am not sure what the underlying cause is but it's probably no coincidence that we have the same error in two similar components on the same exact statement (with the attr.aria-labelledby).

Sometimes these things are not always immediately reproducible and we have to go off crash reports and put in defensive coding or fixes simply based off reports that include the call stack, such as this one does.

Since this looks to be literally the same problem, with the same code, just in a different component, why not apply the same fix here to be safe? It was needed in one, why would it not be needed in both?

image

And as I asked in the original post, is there any reason we don't have an updated Stackblitz repro template that uses the latest PrimeNG 16? I can not get 16 to work on StackBlitz, I am unsure why it throws this error.

@psarno I will create a new PR adding the same implementations because you are right, It's the same problem but I don't know why can't replicated. And I'm not decide this things and I don't know why Stackblitz is not working.
I'm not part of PrimeNG Team, only a contributor here trying to help, ok?
Also I don't know If PrimeNG Team will approve it, because again, I tested and I cannot replicated the problem, and you are the only user with that problem. In that issue (#13497), a lot of problem were reporting.

@psarno
Copy link
Author

psarno commented Sep 9, 2023

The error is occuring in the <p-dialog> component as shown by the call stack, but our actual repro scenario is a component setup such as the following:

<p-dialog>
   <ng-template pTemplate="body">
      <p-autoComplete></p-autoComplete>
    </ng-template>
</p-dialog>

That is overly simplified but it shows the control that causes this error to occur.

When we type a character into the <p-autoComplete>, it immediatly throws the
ExpressionChangedAfterItHasBeenCheckedError. Note however, that this is not occuring in the <p-autoComplete>, even though that triggers it to occur ... it is occuring in the <p-dialog> as per the call stack.

@psarno
Copy link
Author

psarno commented Sep 9, 2023

@SoyDiego Understood that you're just a contributor, you are doing a great work with the PRs.

I am also just leaving these notes here so the PrimeNG team can see the logic behind what is going on, if this is ever merged.

@cagataycivici Is there any chance we can get an officially updated Stackblitz template for PrimeNG from your team to version 16? I don't know who to contact about that. The current one is on 14. I have tried to get it working with 16 but run into one particular error I haven't figured out.

@SoyDiego
Copy link
Contributor

SoyDiego commented Sep 9, 2023

The error is occuring in the <p-dialog> component as shown by the call stack, but our actual repro scenario is a component setup such as the following:

<p-dialog>
   <ng-template pTemplate="body">
      <p-autoComplete></p-autoComplete>
    </ng-template>
</p-dialog>

That is overly simplified but it shows the control that causes this error to occur.

When we type a character into the <p-autoComplete>, it immediatly throws the ExpressionChangedAfterItHasBeenCheckedError. Note however, that this is not occuring in the <p-autoComplete>, even though that triggers it to occur ... it is occuring in the <p-dialog> as per the call stack.

I tried with this example too and I couldn't see the problem.
Anyway, I have created a PR here: #13644 applying the same solution as dynamycdialog component (#13538).
Will see If PrimeNG Team approve it or not.

@cetincakiroglu cetincakiroglu added this to the 16.4.2 milestone Sep 30, 2023
@cetincakiroglu cetincakiroglu removed the Status: Needs Triage Issue will be reviewed by Core Team and a relevant label will be added as soon as possible label Sep 30, 2023
@jncalderon
Copy link

@psarno @cetincakiroglu hi guys, I see that this fix become on 16.4.2 release, how I can get before?

@cetincakiroglu
Copy link
Contributor

Hi @psarno,

You can't get before, we'll release it today!

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