refactor(dashboards): move time series fetcing descision to TimeSeries #14858
Conversation
121watts
force-pushed
the
fix/dashboard-page
branch
from
2021fc0
to
3ed53fa
Compare
| @@ -78,22 +80,43 @@ const defaultState = (): State => ({ | |||
| class TimeSeries extends Component<Props & WithRouterProps, State> { | |||
| public static defaultProps = { | |||
| implicitSubmit: true, | |||
| className: 'vis-plot-container', | |||
| } | ||
|
|
||
| public async componentDidUpdate(prevProps: Props) { | ||
| if (this.shouldReload(prevProps)) { | ||
| if (this.shouldReload(prevProps) && this.isIntersecting) { |
There was a problem hiding this comment.
Can this be put in the shouldReload method?
| this.reload() | ||
| } | ||
| } | ||
|
|
||
| public componentWillMount() { |
There was a problem hiding this comment.
| public componentWillMount() { | |
| public componentWillUnmount() { |
| this.observer = new IntersectionObserver(entries => { | ||
| entries.forEach(entry => { | ||
| const {isIntersecting} = entry | ||
| if (!this.isIntersecting && isIntersecting) { |
There was a problem hiding this comment.
Won't this reload a cell every time in scrolls from out of to into view? If true, seems pretty undesirable.
How about queuing reloads into state, then flushing that queue every time the component scrolls into view?
I imagine that working like: any time the component should reload (whether or not it is in view), we set a flag like pendingReload: true or something. When we reload, we set pendingReload: false. Then this line could become
| if (!this.isIntersecting && isIntersecting) { | |
| if (!this.isIntersecting && isIntersecting && this.pendingReload) { |
There was a problem hiding this comment.
I'm realizing that the existing behavior is reloading the cell when it scrolls from out of view to in view as well, but I still think we should fix it.
| @@ -238,19 +215,6 @@ class DashboardPage extends Component<Props, State> { | |||
| await getDashboard(params.dashboardID) | |||
| } | |||
|
|
|||
| private inView = (cell: Cell): boolean => { | |||
|
Why did we do this? |
|
@ebb-tide Daniel & myself have laid out some changes to dashboards we want to do, such as moving the presentational elements into Clockface. Some of these changes unblock all that. The PR started out as an investigation and ended up as usable code |
#14858) * refactor(dashboards): move time series fetcing descision to TimeSeries * fix: styles and wont refetch already rendered data
What
IntersectionObserverlogic out of theCellcomponent and into theTimeSeriescomponentDashboardPageand moved the "inView" logic intoTimeSeriesWhy
Cellshould fetch data in one place@influxdata/clockfaceFancyScrollbarsReactGridLayoutand styles into presentational components that can be shared across multiple projects (like Quartz)