[APM] Removes refresh logic from date picker #33278
Conversation
💔 Build Failed |
| const { push, replace } = this.props.history; | ||
| const update = currentSearch === nextSearch ? replace : push; | ||
|
|
||
| update({ ...this.props.location, search: nextSearch }); |
There was a problem hiding this comment.
Do we need to do the update, if they are the same? Would the following cause issues?
if(currentSearch !== nextSearch) {
this.props.history.push({ ...this.props.location, search: nextSearch });
}There was a problem hiding this comment.
Yeah we still need to do the update because it re-calculates the relative range.
| @@ -39,25 +39,10 @@ class DatePickerComponent extends React.Component<Props> { | |||
|
|
|||
| public componentDidMount() { | |||
| this.dispatchTimeRangeUpdate(); | |||
There was a problem hiding this comment.
Does the DatePickerComponent still need to be stateful?
My line of thought: EUI date picker is just ui controls for changing the date. If we wanted (and our users were super geeky), we should be able to remove the ui controls, and users should still be able to control the date via the url (the primary set of controls).
Btw. This is not super relevant for this PR - just thinking out loud :)
There was a problem hiding this comment.
Cool, not sure if I follow yet. Are you saying other parts of the app should be able to control the date via URL? Because we already do that in the charts for the drag to select a time range, and it all works -- are you thinking that because of that, the url manipulation shouldn't happen in our date picker wrapper?
There was a problem hiding this comment.
Are you saying other parts of the app should be able to control the date via URL
I'm saying that the rest of the application should be able to work properly and read the date from the url, without the date picker.
Right now, if you removed the date picker the rest of the app wouldn't work, right?
Because we already do that in the charts for the drag to select a time range, and it all works
The chart manipulates the url (I hope). So should the date picker. Only exception is the refresh button, which should invoke a refresh action in redux.
|
retest |
💔 Build Failed |
…updated url is the same except for encoding
jasonrhodes
force-pushed
the
apm-remove-datepicker-refresh_33216
branch
from
e5b1e44 to
334d88b
Compare
💔 Build Failed |
|
retest |
💚 Build Succeeded |
…updated url is the same except for encoding (elastic#33278)
Closes #33216
Blocked by elastic/eui#1732
Summary
onRefreshprop in EUI'sSuperDatePickeronRefreshfunctionalityhistory.replace()instead ofhistory.push()to prevent this problem