[Security Solution] [Endpoint] Search params are reset when opening flyout in policy details artifact tab #127718
Conversation
dasansol92
added
release_note:skip
dasansol92
changed the title
|
@elasticmachine merge upstream |
…ng_flyout_in_policy_details_artifact_tab-3318
|
@elasticmachine merge upstream |
…ng_flyout_in_policy_details_artifact_tab-3318
…ng_flyout_in_policy_details_artifact_tab-3318
…n_policy_details_artifact_tab-3318' of github.com:dasansol92/kibana into fix/olm-search_params_are_reseted_when_opening_flyout_in_policy_details_artifact_tab-3318
| @@ -64,6 +64,20 @@ export const useUrlPagination = (): UrlPagination => { | |||
| const location = useLocation(); | |||
| const history = useHistory(); | |||
| const { urlParams, toUrlParams } = useUrlParams(); | |||
|
|
|||
| useEffect(() => { | |||
There was a problem hiding this comment.
@paul-tavares let me know your thoughts on this. I decided to put this at this level as can be useful when using this hook in other places when migrating the list pages
There was a problem hiding this comment.
@dasansol92 - Yeah, having it here would address the problem, but it feels really wrong (to me 😄 ) that we are doing a history.replace() in a hook meant to "get" url params - especially when it is being done automatically.
Also, the migration below is from page_index to page, which may not be an accurate mapping - page_index is usually 0 based, where we now want to start using page as 1 based in order to provide better URL UX to the users. (ex. User clicks on page 2, they see 2 in the url)
There was a problem hiding this comment.
That's totally right. Then we can create a specific hook for it and use it in the needed components or move this code directly to the specific component. Thoughts?
|
Pinging @elastic/security-onboarding-and-lifecycle-mgt (Team:Onboarding and Lifecycle Mgt) |
| ); | ||
| } | ||
| // Apply this only when on mount this hook | ||
| // eslint-disable-next-line react-hooks/exhaustive-deps |
There was a problem hiding this comment.
🤢 😆
can we avoid this eslint disable rule?
There was a problem hiding this comment.
Is there a way to have an empty hook deps without a eslint error? I'm trying to run this only when component mounts
There was a problem hiding this comment.
maybe pair it with isMounted() and just allow it run on MOunt?
Another option is to use a useRef() and set the reference.current to a function, then use that here. but it is weir :)
…o adds a unit test for it
|
@elasticmachine merge upstream |
…ng_flyout_in_policy_details_artifact_tab-3318
| .replaceAll('page_size', 'pageSize') | ||
| .replaceAll( | ||
| `page_index=${urlParams.page_index}`, | ||
| `page=${Number(urlParams.page_index) + 1}` |
There was a problem hiding this comment.
Maybe add this note to the JSDoc for this hook: **NOTE**: This hook will also increment the page_indexby1sincepage is now one-based
…n_policy_details_artifact_tab-3318' of github.com:dasansol92/kibana into fix/olm-search_params_are_reseted_when_opening_flyout_in_policy_details_artifact_tab-3318
💚 Build SucceededMetrics [docs]Module Count
Async chunks
History
To update your PR or re-run it, just comment with: |
Summary
Before:
After:
For maintainers