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

[Bug] Modal rerender for every keyup/keydown event #459

Closed
BeyondDefinition opened this issue Oct 11, 2022 · 2 comments · Fixed by #461
Closed

[Bug] Modal rerender for every keyup/keydown event #459

BeyondDefinition opened this issue Oct 11, 2022 · 2 comments · Fixed by #461
Labels
Bug Something isn't working Triage Issue needs to be triaged

Comments

@BeyondDefinition
Copy link

Describe the bug
When using [CascadingParameter] ModalInstance, apparently the onkeyup and onkeydown handlers in FocusTrap sets the CascadedValue every time any keyup/keydown DOM event is generated in a child component, which rerenders the component twice for every key press.

This causes OnParametersSet to be called in Typeahead. If you remove these 2 event handlers, the Typeahead component also works as intended. See Blazored/Typeahead#288

It does seem to generate some overhead. Perhaps it's possible to disable event propagation?

To Reproduce
Steps to reproduce the behavior:

  1. Create new VS project.
  2. Follow the guide to add Modal
  3. Make sure to add [CascadingParameter] ModalInstance and manually implement the getter + setter.
  4. Trace the calls to the setter, for example with a breakpoint or Debugger.Log.

Expected behavior
Rerender only when the component state has actually changed, not twice for every key press.

Hosting Model (is this issue happening with a certain hosting model?):

  • Blazor Server
@chrissainty
Copy link
Member

chrissainty commented Oct 11, 2022

Thanks for opening this issue @BeyondDefinition. I think the solution to this is pretty simple, currently the BlazoredModalInstance value that is cascaded down isn't fixed. I've just done a quick test and setting it to fixed will resolve this.

<CascadingValue Value="this" IsFixed="true">
    @Content
</CascadingValue>

@BeyondDefinition
Copy link
Author

That indeed solves it! Thanks!
I wish it was always as easy as adding IsFixed="true" to fix an issue!
Can you please include this improvement in the next release?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working Triage Issue needs to be triaged
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants