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

Dropdown: Fixes double call to onVisibilityChange #87607

Merged
merged 3 commits into from May 10, 2024

Conversation

torkelo
Copy link
Member

@torkelo torkelo commented May 10, 2024

Noticed double calls to onVisbilityChange if that callback is not memoized (https://github.com/grafana/grafana/blob/main/public/app/features/dashboard-scene/scene/NavToolbarActions.tsx#L163)

While it should be memoized I do not think this double call behavior is not correct, the callback should only be called when the visibility changes not when the callback changes.

Using a ref for the callback fixes it, but there are probably other ways, not sure which is best.

@torkelo torkelo requested a review from a team as a code owner May 10, 2024 10:53
@torkelo torkelo requested review from joshhunt, L-M-K-B and ashharrison90 and removed request for a team May 10, 2024 10:53
@grafana-delivery-bot grafana-delivery-bot bot added this to the 11.1.x milestone May 10, 2024
@torkelo torkelo added type/bug no-backport Skip backport of PR no-changelog Skip including change in changelog/release notes labels May 10, 2024
@torkelo torkelo requested a review from joshhunt May 10, 2024 11:25
@torkelo torkelo merged commit 459673b into main May 10, 2024
18 checks passed
@torkelo torkelo deleted the fix-dropdown-double-call branch May 10, 2024 12:06
obetomuniz added a commit that referenced this pull request May 10, 2024
…-tracker

* gops-configuration-tracker: (56 commits)
  SAML: add referemce to azure ad limitations (#87571)
  Update dependency glob to v10.3.14
  Chore: Updated go.work.sum file (#87622)
  Update dependency @types/lodash to v4.17.1 (#87419)
  AdHocFilters: Use queries in ad hoc filters api calls (#87624)
  TimeSeries: Improve keyboard focus and fix spacebar override (#86848)
  Dropdown: Fixes double call to onVisibilityChange (#87607)
  [DOC] Adds missing link to Pyroscope core data source (#85493)
  Docs: Add Configuring Elasticsearch documentation for required privileges (#87593)
  Update dependency eslint-plugin-jsdoc to v48.2.4
  Chore: Add login protection when user is trying different uppercase letters (#87588)
  Explore: lookup datasource by name when present in legacy URLs (#85222)
  Update dependency @prometheus-io/lezer-promql to v0.52.0 (#87554)
  Update dependency systemjs to v6.15.1 (#87594)
  Chore: Remove deprecated re-exported template variable types (#87459)
  Chore: Use getNextRefId instead of deprecated getNextRefIdChar (#87460)
  Chore: Remove use of deprecated method in AC code (#87541)
  Chore:: Update authlib version (#87603)
  Alerting: Reduce number of request fetching rules in the dashboard view using rtkq (#86991)
  refactor: rename variable
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/frontend no-backport Skip backport of PR no-changelog Skip including change in changelog/release notes type/bug
Projects
Status: 🚀 Done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants