[Discover][SavedSearch] Fix default rowsPerPage for Dashboard panels #189717
Conversation
jughosta
added
release_note:skip
|
/ci |
| const rowsPerPage$ = new BehaviorSubject<number | undefined>( | ||
| initialState.rowsPerPage ?? defaultRowsPerPage | ||
| ); |
There was a problem hiding this comment.
@Heenawter Is it a right way of fixing or it should be changed somewhere else?
There was a problem hiding this comment.
This definitely works!! Did a quick test and, because of how we are serializing this value, rowsPerPage will always be saved as part of the panel when it is overwritten by defaultRowsPerPage unless defaultRowsPerPage is equal to the value of the saved search rows per page. Is this desired? if not, I think you have two options:
- You leave this as-is and add a special case for
rowPerPagein your serialize so that you only add it to the serialized panel if it's not equal to the saved searchrowsPerPageor the default rows per page. (this feels like overkill to me personally) - You overwrite in the component instead, so that this value never gets saved. (this seems cleanest?)
But again, if you're fine with the current behaviour, I personally don't see anything wrong with it :)
There was a problem hiding this comment.
@Heenawter Thanks for the suggestions! I will look then into switching to (2).
|
Pinging @elastic/kibana-data-discovery (Team:DataDiscovery) |
|
/ci |
💛 Build succeeded, but was flaky
Failed CI StepsMetrics [docs]Module Count
Async chunks
History
To update your PR or re-run it, just comment with: cc @jughosta |
Summary
This PR fixes an issue with
rowsPerPageAdvanced Setting: after the refactoring it was ignored.To test:
Change
rowsPerPageon Advanced Setting page, navigate to Dashboard and add a saved search panel. It should use the configured value by default. The default value can be overwritten by custom panel settings or saved search own settings.Checklist