fix: issue while switching between views #6200
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id> |
|
Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id> |
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
There was a problem hiding this comment.
👍 Looks good to me! Reviewed everything up to 34a4028 in 18 seconds
More details
- Looked at
40lines of code in1files - Skipped
0files when reviewing. - Skipped posting
2drafted comments based on config settings.
1. frontend/src/container/ExplorerOptions/ExplorerOptions.tsx:352
- Draft comment:
The order of function calls inhandleSelecthas been changed. Ensure thatupdatePreservedViewInLocalStorageandupdateOrRestoreSelectColumnsare intended to be called beforeonMenuItemSelectHandler. This change seems logical as it updates the local storage and restores columns before handling the menu item selection. - Reason this comment was not posted:
Confidence changes required:50%
The PR changes the order of function calls inhandleSelect. This change is logical because it updates the local storage and restores columns before handling the menu item selection, which seems to be the intended behavior.
2. frontend/src/container/ExplorerOptions/ExplorerOptions.tsx:349
- Draft comment:
Avoid using inline styles in React components. Consider using external stylesheets, CSS classes, or styled components instead. This applies to other inline styles in this file as well. - Reason this comment was not posted:
Comment was on unchanged code.
Workflow ID: wflow_jHIk9orPcW6GoUY2
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
ahmadshaheer
force-pushed
the
6187-selecting-view-in-explorer-options-does-not-switch-to-view
branch
from
34a4028
to
0319da3
Compare
|
Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id> |
There was a problem hiding this comment.
👍 Looks good to me! Reviewed everything up to 0319da3 in 17 seconds
More details
- Looked at
44lines of code in1files - Skipped
0files when reviewing. - Skipped posting
4drafted comments based on config settings.
1. frontend/src/hooks/useUrlQueryData.ts:4
- Draft comment:
Consider using consistent import paths throughout the codebase. The import path foruseUrlQuerywas changed from an absolute to a relative path. Ensure this is consistent with other imports. - Reason this comment was not posted:
Confidence changes required:20%
The import statement foruseUrlQuerywas changed from a relative path to an absolute path, which is a minor change but should be consistent with the rest of the codebase.
2. frontend/src/hooks/useUrlQueryData.ts:14
- Draft comment:
The order of dependencies in theuseMemohook was changed. Ensure this order is consistent with other similar hooks in the codebase. - Reason this comment was not posted:
Confidence changes required:20%
TheuseMemodependency array was reordered, which is a minor change but should be consistent with the rest of the codebase.
3. frontend/src/hooks/useUrlQueryData.ts:35
- Draft comment:
TheuseCallbackdependency array was modified to removeurlQuery. Ensure this change does not affect the expected behavior of theredirectWithQueryfunction. - Reason this comment was not posted:
Confidence changes required:50%
TheuseCallbackdependency array was updated to removeurlQuery, which is a significant change. This could affect the behavior of theredirectWithQueryfunction.
4. frontend/src/hooks/useUrlQueryData.ts:1
- Draft comment:
Avoid using thecomponent/index.tsxfile structure approach, as it makes it difficult to debug and find components using global search tools like VS Code. Ensure that the file structure is clear and components are easily searchable. - Reason this comment was not posted:
Confidence changes required:50%
The code does not violate any of the specified rules. It uses hooks and does not involve any design tokens, inline styles, or ClickHouseReader interface issues.
Workflow ID: wflow_uSKP9VbRfAukCWmR
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on 88c3fa8 in 15 seconds
More details
- Looked at
24lines of code in1files - Skipped
0files when reviewing. - Skipped posting
1drafted comments based on config settings.
1. frontend/src/hooks/useUrlQueryData.ts:27
- Draft comment:
Consider usinglocation.searchinstead ofwindow.location.searchfor consistency with thelocationobject fromuseLocation(). - Reason this comment was not posted:
Comment was on unchanged code.
Workflow ID: wflow_yH6g6PJUhqDGon8S
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
vikrantgupta25
approved these changes
Oct 17, 2024
SagarRajput-7
approved these changes
Oct 21, 2024
YounixM
approved these changes
Oct 28, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
the root cause of #6187 was, using
useUrlQueryto get the current URL's search params, we used to get the staleviewName, therefore, it would update the selectColumns with the oldviewNamebeing selected. thus it would keep the old view selected.by creating
URLSearchParamsinside theredirectWithQuery, we ensure that we're working with the most up-to-date URL state.Related Issues / PR's
close #6187
close https://github.com/SigNoz/engineering-pod/issues/1913
Screenshots
NA
Affected Areas and Manually Tested Areas
Affecter areas:
Tested:
Important
Fixes URL query handling in
useUrlQueryDataby usingURLSearchParamsfor up-to-date query state and corrects import path foruseUrlQuery.useUrlQueryDataby usingURLSearchParamsto ensure up-to-date query state.redirectWithQueryto construct URLs with current pathname and updated query string.This description was created by
for 88c3fa8. It will automatically update as commits are pushed.