-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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 highlight config so it doesn't require a refresh to apply #10346
Conversation
This looks pretty good. I did find one edge case. If you add a saved search from a pre 5.3 installation to a dashboard you'll still get highlighting even if |
Good catch, @Bargs. |
@Bargs Could you take another look? Thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
jenkins, test this |
* Acknowledge doc_table:highlight config even after init * Use highlightAll on dashboard
Backported to 5.3 in 3b34f16. |
@lukasolson I had discover and advanced settings open in two separate tabs(chrome latest). Then I tried setting the value of doc_table:highlight and looked at the results in discover. The settings didn't get applied till I refreshed the page. Is this expected. cc @Bargs this works fine otherwise. What we saw together in zoom - I couldn't reproduce it in local. Thanks! |
* Acknowledge doc_table:highlight config even after init * Use highlightAll on dashboard
Prior to this PR, when you would go into the advanced settings and change the
doc_table:highlight
tofalse
(ortrue
), it would require refreshing the page before it would be applied. This was confusing.This PR fixes this behavior such that the config is checked each time we go to apply highlighting instead of only when the page loads, and adds a test surrounding this behavior.
Fixes #7796.