NETOBSERV-783 multiple page loads with quick filters #273
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
| @@ -49,7 +49,7 @@ class NetflowTrafficParent extends React.Component<Props, State> { | |||
| return this.props.children; | |||
| } | |||
| // else render default NetworkTraffic | |||
| return <NetflowTraffic />; | |||
| return <NetflowTraffic forcedFilters={null} />; | |||
There was a problem hiding this comment.
why do we need a distinction null vs undefined?
There was a problem hiding this comment.
Coming from netflow-tab, the prop will be undefined before being set.
As we force null in netflow-traffic-parent, we make the distinction between the loading state from tabs and the null state from page to skip a tick before confing loaded
| // skip tick while forcedFilters & config are not loaded | ||
| // this check ensure tick will not be called during init | ||
| // as it's difficult to manage react state changes | ||
| if (!initState.current.includes('forcedFiltersLoaded') || !initState.current.includes('configLoaded')) { |
There was a problem hiding this comment.
I think it would be better to have a union type for initState such as array of 'forcedFiltersLoaded' | 'configLoaded' | ... (e.g. to avoid typos)
|
/ok-to-test |
openshift-ci
bot
added
the
ok-to-test
|
New image: ["quay.io/netobserv/network-observability-console-plugin:6927299"]. It will expire after two weeks. |
|
@jpinsonneau - I see the "default" quick filters are not coming selected with this image, is that intentional? |
|
I can confirm stability of tests have improved vastly with this change, where it loads page only once, currently without default quick filters - I am not sure if we should have kept page load with default quick filters instead? |
Good catch @memodi ! That was not intentionnal. |
github-actions
bot
removed
the
ok-to-test
|
/ok-to-test |
openshift-ci
bot
added
the
ok-to-test
|
New image: ["quay.io/netobserv/network-observability-console-plugin:c3325a5"]. It will expire after two weeks. |
This image works for default filters at first start on my side 🥳 |
|
/qe-approved |
|
/approve |
github-actions
bot
removed
the
ok-to-test
|
no changes, just fixed merge conflict |
This fix avoid tick being called multiple times at start.
Code changes:
config/forcedFiltersuseEffectforcedFiltersare disabled to differenciate with undefinedinitStateas string array to let developpers better understand what has been done, in which order and allow future states