-
Notifications
You must be signed in to change notification settings - Fork 410
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 #3930 short height of query panel filter when opened in dashboard #4105
Conversation
…ed in dashboard
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.
Hi, Please add a test for this fix
appliedFilterSelector, | ||
storedFilterSelector, | ||
(state) => state && state.query && state.query.isLayerFilter, | ||
selectedLayerLoadingErrorSelector | ||
], (enabled, groups, settings, querypanelEnabled, layout, appliedFilter, storedFilter, advancedToolbar, loadingError) => ({ | ||
], (enabled, groups, settings, querypanelEnabled, layoutHeight, dashboardAvailable, appliedFilter, storedFilter, advancedToolbar, loadingError) => ({ |
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.
Why did you changed the name here from layout to layoutHeight?
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.
It was misleading; the property contains only layout height.
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.
ok
Adding a test for this plugin is overkill for the changes I only did. |
ok |
@kasongoyo please backport this to 2019.02.xx |
Description
Fix the query panel height when opened in dashboard context
Issues
Please check if the PR fulfills these requirements
What kind of change does this PR introduce? (check one with "x", remove the others)
What is the current behavior? (You can also link to an open issue here)
see #3930
What is the new behavior?
Does this PR introduce a breaking change? (check one with "x", remove the other)
If this PR contains a breaking change, please describe the impact and migration path for existing applications: ...
Other information: