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

[Widget Params] Bug: widget oblivious to updated static value #3379

Closed
ranbena opened this issue Jan 31, 2019 · 11 comments
Closed

[Widget Params] Bug: widget oblivious to updated static value #3379

ranbena opened this issue Jan 31, 2019 · 11 comments
Assignees

Comments

@ranbena
Copy link
Contributor

ranbena commented Jan 31, 2019

Originally reported by @arikfr in #3334 (comment).

STR:

  1. Go to widget with static parameter, click "edit parameters".
  2. Change value of parameter, click "save".

Expected - Widget auto refreshes with new value.
Actual - Widget remains with previous value.

Clicking the widget refresh button updates it. Just need to make it automatic on change.

@ranbena
Copy link
Contributor Author

ranbena commented Feb 1, 2019

@kravets-levko is this something you have time to fix?

@ranbena
Copy link
Contributor Author

ranbena commented Feb 4, 2019

@arikfr @kravets-levko do you think this would do it? a843b92

@kravets-levko
Copy link
Collaborator

Indeed it will work. I'm just not sure if it solves described issue - it will refresh widget on any parameter mappings change, not only static values 🤔

@ranbena
Copy link
Contributor Author

ranbena commented Feb 4, 2019

Indeed it will work. I'm just not sure if it solves described issue - it will refresh widget on any parameter mappings change, not only static values 🤔

Makes sense anyway, no? If any param changed, refresh visualization.

@kravets-levko
Copy link
Collaborator

Makes sense anyway, no?

Probably yes 🙂

@kravets-levko
Copy link
Collaborator

@arikfr WDYT?

@arikfr
Copy link
Member

arikfr commented Feb 5, 2019

I think it makes sense -- like the case where you change a mapping from one dashboard param to another.

So as long as it runs when mapping changed, I'm good with this.

@ranbena
Copy link
Contributor Author

ranbena commented Feb 5, 2019

In this pr it refreshes on save button click. Which means, also param label renaming. Would you like to filter that case out?

Is refresh such an expensive action that we'd rather avoid any needless calls to it?

@arikfr
Copy link
Member

arikfr commented Feb 5, 2019

Is refresh such an expensive action that we'd rather avoid any needless calls to it?

Sometimes it is, sometimes it isn't...

If it's easy to call it only when really needed, let's do it. Otherwise, let's fix the bug.

@susodapop
Copy link
Contributor

In this pr it refreshes on save button click.

In my experience this could be a real dealbreaker for trying out the functionality. Some of my dashboards can take north of a minute to update. It feels weird to make changes to the title of a parameter and have it refresh the relevant widgets. Refresh should only happen when absolutely necessary IMO.

@ranbena
Copy link
Contributor Author

ranbena commented Feb 16, 2019

Made it so only parameter value changes trigger an automatic refresh.
Title changes won't, even param type change won't.
#3445

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

No branches or pull requests

4 participants