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
Do not trigger slideChange on the initial render. #24717
Do not trigger slideChange on the initial render. #24717
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A bit confused as to why triggering the event on the initial render is what's causing the error in the issue. Could you clarify that?
* avoid triggering a slideChange on the initial render. | ||
* @private {boolean} | ||
*/ | ||
this.skipNextSlideChange_ = true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not possible to use the check that currentIndex_
is null instead of adding an instance variable here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good call, made the change and also fixed #24701 in the process.
The problem is that on initial render, we trigger the slideChange with low trust (since the user did not interact) and we do not want it to be used for As a result, the code that runs the action complains that you are trying to do a high trust required action with a low trust source. |
@wassgha @sparhami I noticed this PR was submitted without owners approval. Should the For reference, see the current owners tree |
I created #24739 to update the relevant owners. |
NaN
to an actual index, so that will still notify.Closes #24706
Fixes #24701