feat(URL): Share link with search parameters #173
Conversation
WalkthroughThe pull request enhances the Changes
Possibly Related PRs
Suggested Reviewers
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/components/CentralContainer/Sidebar/SidebarTabs/SearchTabPanel/index.tsx (1)
80-86: Consider omitting the empty object parameter.The empty object passed as
hashParamsUpdatesis unnecessary.const handleShareButtonClick = () => { copyPermalinkToClipboard({ queryString: queryString, isCaseSensitive: getIsCaseSensitive(queryOptions), isRegex: getIsRegex(queryOptions), - }, {}); + }); };src/contexts/StateContextProvider.tsx (1)
459-468: Simplify the search parameter condition.The condition is complex and could be simplified. Also, consider moving the URL parameter reset to a cleanup function.
useEffect(() => { - if (URL_SEARCH_PARAMS_DEFAULT.queryString !== queryString && URL_SEARCH_PARAMS_DEFAULT.isCaseSensitive !== isCaseSensitive && URL_SEARCH_PARAMS_DEFAULT.isRegex !== isRegex) { + const hasSearchParams = queryString !== URL_SEARCH_PARAMS_DEFAULT.queryString || + isCaseSensitive !== URL_SEARCH_PARAMS_DEFAULT.isCaseSensitive || + isRegex !== URL_SEARCH_PARAMS_DEFAULT.isRegex; + + if (hasSearchParams) { startQuery({queryString, isCaseSensitive, isRegex}); } - updateWindowUrlSearchParams({ - [SEARCH_PARAM_NAMES.QUERY_STRING]: URL_SEARCH_PARAMS_DEFAULT.queryString, - [SEARCH_PARAM_NAMES.IS_CASE_SENSITIVE]: URL_SEARCH_PARAMS_DEFAULT.isCaseSensitive, - [SEARCH_PARAM_NAMES.IS_REGEX]: URL_SEARCH_PARAMS_DEFAULT.isRegex, - }); + + return () => { + updateWindowUrlSearchParams({ + [SEARCH_PARAM_NAMES.QUERY_STRING]: URL_SEARCH_PARAMS_DEFAULT.queryString, + [SEARCH_PARAM_NAMES.IS_CASE_SENSITIVE]: URL_SEARCH_PARAMS_DEFAULT.isCaseSensitive, + [SEARCH_PARAM_NAMES.IS_REGEX]: URL_SEARCH_PARAMS_DEFAULT.isRegex, + }); + }; }, [isFileLoaded]);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/components/CentralContainer/Sidebar/SidebarTabs/SearchTabPanel/index.tsx(4 hunks)src/contexts/StateContextProvider.tsx(4 hunks)src/contexts/UrlContextProvider.tsx(2 hunks)src/typings/url.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
src/contexts/UrlContextProvider.tsx (1)
Pattern **/*.{cpp,hpp,java,js,jsx,ts,tsx}: - Prefer false == <expression> rather than !<expression>.
src/typings/url.ts (1)
Pattern **/*.{cpp,hpp,java,js,jsx,ts,tsx}: - Prefer false == <expression> rather than !<expression>.
src/components/CentralContainer/Sidebar/SidebarTabs/SearchTabPanel/index.tsx (1)
Pattern **/*.{cpp,hpp,java,js,jsx,ts,tsx}: - Prefer false == <expression> rather than !<expression>.
src/contexts/StateContextProvider.tsx (1)
Pattern **/*.{cpp,hpp,java,js,jsx,ts,tsx}: - Prefer false == <expression> rather than !<expression>.
🔇 Additional comments (9)
src/typings/url.ts (2)
6-8: LGTM! The new search parameter names are well-defined.The added enum values follow consistent naming conventions and use appropriate camelCase for URL parameters.
17-19: LGTM! The interface properties are well-typed.The added properties use appropriate types and maintain type safety through computed property names.
src/components/CentralContainer/Sidebar/SidebarTabs/SearchTabPanel/index.tsx (3)
16-16: LGTM! The imports support the new sharing functionality.The added imports are necessary and well-organized.
Also applies to: 21-24
72-72: LGTM! The URL context integration is well-implemented.The context usage and state initialization follow React best practices with proper null handling.
Also applies to: 75-75
131-136: LGTM! The share button UI is well-integrated.The button follows consistent patterns and provides clear user feedback through the tooltip.
src/contexts/UrlContextProvider.tsx (1)
27-29: LGTM! The default parameters are consistently defined.The new search parameters follow the established pattern and ordering.
src/contexts/StateContextProvider.tsx (3)
249-249: LGTM! The URL context usage is complete.All necessary search parameters are extracted in a consistent order.
254-255: LGTM! The file loading state is well-managed.The state is properly initialized and updated in the file info handler.
Also applies to: 309-309
424-425: LGTM! The query state reset is appropriate.Query results and progress are properly reset when loading a new file.
|
(no rush; whenever you have time; preferably after your vacation) |
# Conflicts: # src/typings/url.ts
…s after file is loaded
# Conflicts: # src/components/CentralContainer/Sidebar/SidebarTabs/SearchTabPanel/index.tsx
…nk with params button
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/components/CentralContainer/Sidebar/SidebarTabs/SearchTabPanel/index.tsx (3)
53-53: Consider restructuring to avoid disabling the max-lines-per-function lint rule.
If feasible, reduce the function’s size by extracting logic into smaller helper functions, which can improve maintainability and readability.
118-119: Adhere to the coding guideline for negation checks.
Replace!queryIsCaseSensitivewithfalse == queryIsCaseSensitivefor consistency.-handleQuerySubmit({queryIsCaseSensitive: !queryIsCaseSensitive}); -setQueryIsCaseSensitive(!queryIsCaseSensitive); +handleQuerySubmit({queryIsCaseSensitive: false == queryIsCaseSensitive}); +setQueryIsCaseSensitive(false == queryIsCaseSensitive);
123-124: Maintain consistency for the regex negation as well.
Similarly, usefalse == queryIsRegexinstead of!queryIsRegex.-handleQuerySubmit({queryIsRegex: !queryIsRegex}); -setQueryIsRegex(!queryIsRegex); +handleQuerySubmit({queryIsRegex: false == queryIsRegex}); +setQueryIsRegex(false == queryIsRegex);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/components/CentralContainer/Sidebar/SidebarTabs/SearchTabPanel/index.tsx(8 hunks)src/contexts/StateContextProvider.tsx(4 hunks)src/contexts/UrlContextProvider.tsx(4 hunks)src/services/LogFileManager/index.ts(2 hunks)src/typings/query.ts(1 hunks)src/typings/url.ts(1 hunks)src/typings/worker.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/contexts/StateContextProvider.tsx
- src/typings/url.ts
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}`: - Prefer `false == ...
**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}: - Preferfalse == <expression>rather than!<expression>.
src/typings/worker.tssrc/typings/query.tssrc/services/LogFileManager/index.tssrc/contexts/UrlContextProvider.tsxsrc/components/CentralContainer/Sidebar/SidebarTabs/SearchTabPanel/index.tsx
🪛 GitHub Check: lint-check
src/components/CentralContainer/Sidebar/SidebarTabs/SearchTabPanel/index.tsx
[failure] 81-81:
This line has a length of 109. Maximum allowed is 100
[failure] 86-86:
React Hook useEffect has missing dependencies: 'startQuery', 'urlQueryIsCaseSensitive', and 'urlQueryIsRegex'. Either include them or remove the dependency array
🪛 GitHub Actions: lint
src/components/CentralContainer/Sidebar/SidebarTabs/SearchTabPanel/index.tsx
[warning] 81-81: This line has a length of 109. Maximum allowed is 100 @stylistic/max-len
🔇 Additional comments (22)
src/typings/worker.ts (1)
115-116: LGTM! Parameter renaming improves code clarity.The renaming of parameters to
queryIsCaseSensitiveandqueryIsRegexenhances code readability by explicitly indicating these parameters are related to query operations.src/typings/query.ts (1)
3-4: LGTM! Consistent parameter naming improves maintainability.Renaming these properties to
queryIsCaseSensitiveandqueryIsRegexaligns with the changes in worker.ts, ensuring consistency across the codebase.src/services/LogFileManager/index.ts (3)
295-296: LGTM! JSDoc updates match parameter renaming.The JSDoc parameter descriptions have been correctly updated to reflect the renamed parameters.
298-298: Function signature properly updated to match type changes.The method signature has been updated to use the new parameter names, maintaining consistency with the type changes.
312-317: LGTM! Variable references updated to match new parameter names.The internal code has been appropriately updated to reference the new parameter names.
src/contexts/UrlContextProvider.tsx (4)
34-36: LGTM! New default hash parameters added for query sharing.The addition of these parameters to the default object enables the sharing feature for search parameters.
107-107: LGTM! Condition properly handles false boolean values.The condition now correctly removes parameters when their value is
false, which aligns with the expected behavior for boolean toggle parameters.
187-190: Boolean type conversion is required for URL parameters.The
UrlSearchParamsinterface may define boolean types, but URLSearchParams stores them as strings. Type conversion is necessary for type safety.
221-235: LGTM! Proper handling of query parameters from URL hash.The implementation correctly retrieves and type-converts the query parameters from the URL hash. The string to boolean conversion is properly handled using string comparison.
src/components/CentralContainer/Sidebar/SidebarTabs/SearchTabPanel/index.tsx (13)
3-3: No concerns regarding the import ofuseEffect.
It is properly added and necessary for the upcoming effect logic.
15-15: The new icon import is a beneficial addition.
ImportingShareIconis straightforward and aligns with the sharing feature.
20-25: Appropriate imports fromUrlContextProvider.
They are correctly declared and will be useful for URL manipulation and sharing.
30-33:UI_ELEMENTandUI_STATEusage is correct.
These are likely being used in the effect and logic with no apparent issues.
56-60: DestructuringqueryString,queryIsCaseSensitive,queryIsRegexis implemented correctly.
No concerns here; it cleanly retrieves values from theUrlContext.
63-64: State initialization for case sensitivity and regex flags is clear.
Setting both tofalseby default is logical.
94-101:handleShareButtonClickfunction is straightforward.
Copying the permalink to the clipboard appears well-implemented for sharing the link.
105-106:handleQuerySubmitintegration looks correct.
Referencing the new case-sensitivity and regex state flags aligns with the updated logic.
130-130: No concerns with the updated panel container line.
The panel’s setup remains intact.
134-151: New share button in the panel UI is well integrated.
It complements the existing collapse/expand button, enhancing user experience for link sharing.
161-161: Binding the textarea’s value toqueryStringis correct.
Ensures the component reflects the current search input accurately.
170-171:isChecked={queryIsCaseSensitive}logic is fine.
No issues flagged by the new approach for toggling case-sensitivity.
182-183:isChecked={queryIsRegex}logic is also good.
The toggle correctly binds to the updated state for regex mode.
src/components/CentralContainer/Sidebar/SidebarTabs/SearchTabPanel/index.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/components/CentralContainer/Sidebar/SidebarTabs/SearchTabPanel/index.tsx (1)
78-103: Follow coding guidelines for conditional checks.According to the coding guidelines,
false == <expression>should be preferred over!<expression>.- if (null !== urlQueryString) { + if (null !== urlQueryString) {While this particular line already follows the guidelines, ensure all conditional checks throughout the component maintain this style.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/CentralContainer/Sidebar/SidebarTabs/SearchTabPanel/index.tsx(8 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}`: - Prefer `false == ...
**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}: - Preferfalse == <expression>rather than!<expression>.
src/components/CentralContainer/Sidebar/SidebarTabs/SearchTabPanel/index.tsx
🔇 Additional comments (5)
src/components/CentralContainer/Sidebar/SidebarTabs/SearchTabPanel/index.tsx (5)
108-115: Good implementation of sharing functionality.The
handleShareButtonClickmethod correctly captures the current search state and usescopyPermalinkToClipboardto share it.
70-76: Good use of refs to track URL context values.Using refs to track the URL context values helps prevent unnecessary re-renders while still keeping the component responsive to URL changes.
91-96: Good formatting for line length compliance.The multi-line formatting for the hash parameters object effectively prevents line length issues that were flagged in previous reviews.
148-165: Well-structured UI addition for share button.The share button is implemented consistently with the existing UI patterns, using the same component structure as the collapse/expand button.
99-103: Properly handled useEffect dependencies.The useEffect now correctly includes
startQueryin the dependency array, addressing the issue raised in the previous review.
src/components/CentralContainer/Sidebar/SidebarTabs/SearchTabPanel/index.tsx
Show resolved
Hide resolved
src/components/CentralContainer/Sidebar/SidebarTabs/SearchTabPanel/index.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/components/CentralContainer/Sidebar/SidebarTabs/SearchTabPanel/index.tsx (1)
103-107: Consider adding URL query parameters to the dependency array.The useEffect dependency array is missing
urlQueryIsCaseSensitiveandurlQueryIsRegex. Even though you're using refs to track these values, changes to these URL parameters should trigger the effect to ensure the component properly reacts to URL changes.}, [ startQuery, uiState, urlQueryString, + urlQueryIsCaseSensitive, + urlQueryIsRegex, ]);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/components/CentralContainer/Sidebar/SidebarTabs/SearchTabPanel/index.tsx(8 hunks)src/services/LogFileManager/index.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/services/LogFileManager/index.ts
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}`: - Prefer `false == ...
**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}: - Preferfalse == <expression>rather than!<expression>.
src/components/CentralContainer/Sidebar/SidebarTabs/SearchTabPanel/index.tsx
🔇 Additional comments (6)
src/components/CentralContainer/Sidebar/SidebarTabs/SearchTabPanel/index.tsx (6)
112-119: Good implementation of share functionality.The
handleShareButtonClickfunction correctly implements the link sharing feature by callingcopyPermalinkToClipboardwith the current search parameters.
136-137: Toggle implementation using negation operator.According to the coding guidelines,
false == <expression>should be preferred over!<expression>. However, as discussed in previous comments, using!here makes the toggling action more intuitive to understand.
152-169: Well-structured UI for sharing functionality.The addition of the share button in the title area is cleanly implemented with proper tooltips and icon. Good job maintaining consistent UI patterns with the existing collapse/expand button.
95-101: Consider the intent of resetting URL parameters.This code updates the window URL hash parameters to their default values right after starting a query with custom parameters. Is the intention to reset the URL after starting the query? If not, this might counteract the sharing functionality you're implementing.
Please confirm if this is intentional behavior - typically when implementing shareable URLs, you would want the URL to reflect the current search parameters rather than resetting them to defaults.
84-93: Good state initialization from URL parameters.The implementation correctly initializes the search component's state from URL parameters and starts the query with those parameters when the UI is ready.
67-76: Effective use of refs for tracking URL context values.Using refs to track the URL context values between renders is a good approach to maintain values while avoiding unnecessary re-renders.
| @@ -38,21 +51,77 @@ import "./index.css"; | |||
| * | |||
| * @return | |||
| */ | |||
| // eslint-disable-next-line max-lines-per-function | |||
There was a problem hiding this comment.
| // eslint-disable-next-line max-lines-per-function |
We should avoid disabling linting rules whenever possible. Please see if the refactoring PR at #196 properly addresses this violation.
Description
Add support to making queries in the url. Also, a button that generates link to current query is added in
SearchTabPanel.Validation performed
Summary by CodeRabbit
Release Notes
New Features
Improvements
Technical Updates
This update introduces more flexible search functionality and enhances the user experience for sharing search settings.