docs: Simple example of creating a dashboard filter and applying it to dashboard elements #818
Conversation
jkaster
requested review from
joeldodge79,
josephaxisa,
drstrangelooker,
fabio-looker,
jeremytchang and
lanlooker
jkaster
changed the title
| """ | ||
| # Keep track of the current result_maker and add to it, otherwise listeners for other filters would be removed | ||
| current_filterables = element.result_maker.filterables | ||
| for filterable in current_filterables: |
There was a problem hiding this comment.
This is surely largely to do with my inexperience with Python, but I'm confused why we are iterating through current filterables twice, once here and then once on line 75. I am sure your code is doing the right and necessary thing, but as far as readability, I've tried to figure it out a few times and I'm not 100% clear on this point. Either some additional comment, or combining these loops somehow might help? Not sure!
There was a problem hiding this comment.
Great catch. The code was doing the right thing, just not as efficiently as it could have. Originally when I drew up this process, I used one step to get the current filterables, and a second step to then update them. But, this isn't nearly as efficient as doing that all at once. Consolidated these two loops into one per your suggestion.
There was a problem hiding this comment.
The two loops being consolidated in to one makes this so much clearer, I no longer feel lost :)
On lines 66-68, are you essentially just making a clone of the array? If there is a python method to do that, it might be clearer to use it, otherwise this looks good as is!
There was a problem hiding this comment.
Oh great catch. I've consolidated that down into just the one line in line 66. That loop's definitely unnecessary.
|
@fabio-looker - Thanks for the original review, great suggestions. Added a couple small changes described in the above conversations. When you've got a few, would you mind having a look at the new changes? |
|
Hey @fabio-looker - When you get a chance could I get a quick review? Just one small change to condense the copying of the array. |
drstrangelooker
force-pushed
the
main
branch
4 times, most recently
from
9474788 to
5f9930c
Compare
drstrangelooker
force-pushed
the
main
branch
from
95e05d0 to
d62f446
Compare
drstrangelooker
changed the base branch from
main
to
example_of_creating_dashboard_filter
drstrangelooker
merged this pull request
into
looker-open-source:example_of_creating_dashboard_filter
…o dashboard elements (#818) * Pushing new_dashboard_filter to remote * Adding example to create_dashboard_filter, fixes 816 * Rename dashboard_filter file * add new line at end of file * Fixing new line at end of file 🤦🏼♂️ * Condensing loops * Consolidating loops, adding check for matching model and explore * Removing unnecessary loop Co-authored-by: Kevin Dunn <[email protected]>
…o dashboard elements (#818) * Pushing new_dashboard_filter to remote * Adding example to create_dashboard_filter, fixes 816 * Rename dashboard_filter file * add new line at end of file * Fixing new line at end of file 🤦🏼♂️ * Condensing loops * Consolidating loops, adding check for matching model and explore * Removing unnecessary loop Co-authored-by: Kevin Dunn <[email protected]>
Wanted to submit an example of creating a dashboard filter and listeners for dashboard tiles that update based on the filter. This is based on a number of support requests that ask how to do this using Looker's API. We have some community posts about what endpoints to use, but no real working examples.
Developer Checklist ℹ️
Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:
Fixes #816 🦕