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

fix: query header switches behaving incorrectly #95

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

gwdawson
Copy link
Member

@gwdawson gwdawson commented May 2, 2024

a while ago we found a bug where the the header switches were behaving incorrectly when you have more than one visual query. this was fixed in the grafana core and bigquery builders but never fixed here in plugin-ui.

Before the fix:
https://github.com/grafana/plugin-ui/assets/34524710/359b0720-fa2f-42b3-80e3-7ab2c1254585

After the fix:
https://github.com/grafana/plugin-ui/assets/34524710/a76834d0-6aa9-4671-ac1f-da4f038f4b78

@gwdawson gwdawson self-assigned this May 2, 2024
Copy link

@gabor gabor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

@cletter7
Copy link
Contributor

cletter7 commented May 3, 2024

Just curious, the PR title says "add", while in the changes I see only removal. Could you explain please @gwdawson ?

@gwdawson
Copy link
Member Author

gwdawson commented May 3, 2024

Just curious, the PR title says "add", while in the changes I see only removal. Could you explain please @gwdawson ?

@cletter7 yeah the title is outdated i need to update this.

initially this pr changed the id to look something like this id={'sql-filter-${uuidv4()}'} making them unique.
however this created some bad side effects and we found that by removing the id entirely give us a better result.

@gwdawson gwdawson changed the title add unique switch ids to sql header fix: query header switches behaving incorrectly May 3, 2024
@cletter7
Copy link
Contributor

cletter7 commented May 3, 2024

Got it. Just thinking if it is possible that some tests on the consumer side are relying on those ids.

initially this pr changed the id to look something like this id={'sql-filter-${uuidv4()}'} making them unique.
however this created some bad side effects and we found that by removing the id entirely give us a better result.

Might be because you generate a new id on every re-render. Potentially you could generate those ids on initial render only and store them in a local state.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

None yet

3 participants