[ENGG-4727] fix: green dot tab indicator for param and body tabs#3757
[ENGG-4727] fix: green dot tab indicator for param and body tabs#3757nafees87n merged 9 commits intorequestly:masterfrom
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughChanges across apiClient UI components focused on tab labeling, script editor behavior, and key-value row initialization. KeyValueTable now returns a single empty pair with Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
app/src/features/apiClient/screens/apiClient/components/views/components/request/components/KeyValueTable/KeyValueTable.tsx (1)
37-45: Consider refactoring to avoid object mutation.The logic correctly sets
isEnabled: falsefor the initial empty row, which aligns with the PR objective. However, creating an object viacreateEmptyPair()(which setsisEnabled: true) and then mutating it tofalseis less clean than creating the object directly with the desired state.Option 1: Create the object inline
const memoizedData: KeyValuePair[] = useMemo(() => { if (data.length) { return data; } else { - const emptyData = createEmptyPair(); - emptyData.isEnabled = false; - return [emptyData]; + return [{ + id: Date.now(), + key: "", + value: "", + isEnabled: false, + }]; } }, [data, createEmptyPair]);Option 2: Parameterize
createEmptyPairconst createEmptyPair = useCallback( - () => ({ + (isEnabled = true) => ({ id: Date.now(), key: "", value: "", - isEnabled: true, + isEnabled, }), [] ); const memoizedData: KeyValuePair[] = useMemo(() => { if (data.length) { return data; } else { - const emptyData = createEmptyPair(); - emptyData.isEnabled = false; - return [emptyData]; + return [createEmptyPair(false)]; } }, [data, createEmptyPair]);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/src/features/apiClient/screens/apiClient/components/views/components/request/components/KeyValueTable/KeyValueTable.tsx(1 hunks)app/src/features/apiClient/screens/apiClient/components/views/http/components/HttpRequestTabs/HttpRequestTabs.tsx(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
app/src/features/apiClient/screens/apiClient/components/views/http/components/HttpRequestTabs/HttpRequestTabs.tsx (1)
app/src/features/apiClient/hooks/useQueryParamStore.tsx (1)
useQueryParamStore(7-15)
app/src/features/apiClient/screens/apiClient/components/views/components/request/components/KeyValueTable/KeyValueTable.tsx (2)
shared/src/types/entities/apiClient/api.ts (1)
KeyValuePair(212-218)app/src/features/apiClient/types.ts (1)
KeyValuePair(36-42)
🔇 Additional comments (4)
app/src/features/apiClient/screens/apiClient/components/views/http/components/HttpRequestTabs/HttpRequestTabs.tsx (4)
51-53: LGTM: Enabled query params detection.The logic correctly identifies whether any query parameters are enabled using
.some(), which efficiently short-circuits on the first enabled parameter.
96-100: LGTM: Body tab indicator logic.The count logic correctly evaluates whether the request body contains data by checking
body?.length, showing the green dot indicator only when content exists. The conditionalshowDotbased onisRequestBodySupportedappropriately hides the indicator for methods that don't support request bodies.
175-176: LGTM: Dependencies correctly updated.The dependency array properly includes
hasEnabledQueryParamsandpathVariables.lengthto ensure the memoized items update when the enabled state or path variables change.
59-65: Verification complete: Query params store updates correctly reflect enabled state changes.The checkbox toggle properly propagates through the component chain:
- KeyValueTableRow (line 73): Checkbox onChange → calls
save()- KeyValueTable (line 74):
save()→handleUpdatePair()→onChange(updatedPairs)- QueryParamsTable (line 32):
handleUpdateQueryParams()→setQueryParams(updatedPairs)(store update)- HttpRequestTabs (line 51-53): Store selector detects changes → green dot indicator updates
The green dot logic at lines 59-65 correctly displays when
hasEnabledQueryParamsis true (reading from the store), confirming the state management is working as intended.
|
@mustafa-sayyed Please resolve the conflicts. |
|
@rohanmathur91 Edit:
|
|
Hey @rohanmathur91, @dinex-dev |
|
@mustafa-sayyed In #3731 the aim was to show green dot beside pre request script and post response script labels but I do not see those changes in this PR. |
|
Thanks @nafees87n for pointing out that I will implement green dot beside the Pre request Script and Post request Script |
|
@nafees87n Script.-.Made.with.Clipchamp.mp4 |
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
app/src/features/apiClient/screens/apiClient/components/views/components/Scripts/components/ScriptEditor/ScriptEditor.tsx (1)
52-57: Consider simplifying the count logic for clarity.Same suggestion as the Pre-request indicator above - the logic can be simplified:
count={scripts?.postResponse && scripts.postResponse !== DEFAULT_SCRIPT_VALUES.postResponse ? 1 : 0}
🧹 Nitpick comments (2)
app/src/features/apiClient/screens/apiClient/components/views/components/request/components/KeyValueTable/KeyValueTable.tsx (1)
37-45: Avoid mutating the object returned bycreateEmptyPair.Directly mutating
emptyDataat line 42 after creation reduces code clarity. Use the spread operator to overrideisEnabledwithout mutation.Apply this diff:
const memoizedData: KeyValuePair[] = useMemo(() => { if (data.length) { return data; } else { - const emptyData = createEmptyPair(); - emptyData.isEnabled = false; - return [emptyData]; + return [{ ...createEmptyPair(), isEnabled: false }]; } }, [data, createEmptyPair]);app/src/features/apiClient/screens/apiClient/components/views/components/Scripts/components/ScriptEditor/ScriptEditor.tsx (1)
45-48: Consider simplifying the count logic for clarity.The current logic is functionally correct but could be more explicit:
count={scripts?.preRequest && scripts.preRequest !== DEFAULT_SCRIPT_VALUES.preRequest ? 1 : 0}The
.lengthcheck is redundant when you're already comparing against the default value (which is a non-empty string). This simplification makes the intent clearer: show the indicator when a non-default script exists.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
app/src/features/apiClient/screens/apiClient/components/views/components/Scripts/components/ScriptEditor/ScriptEditor.tsx(3 hunks)app/src/features/apiClient/screens/apiClient/components/views/components/Scripts/components/ScriptEditor/scriptEditor.scss(1 hunks)app/src/features/apiClient/screens/apiClient/components/views/components/request/components/ApiClientRequestTabs/components/RequestTabLabel/RequestTabLabel.tsx(1 hunks)app/src/features/apiClient/screens/apiClient/components/views/components/request/components/KeyValueTable/KeyValueTable.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
Learnt from: mustafa-sayyed
Repo: requestly/requestly PR: 3723
File: app/src/features/apiClient/screens/apiClient/components/views/http/components/HttpRequestTabs/HttpRequestTabs.tsx:146-151
Timestamp: 2025-10-22T22:57:45.959Z
Learning: In the API Client Scripts tab (app/src/features/apiClient/screens/apiClient/components/views/http/components/HttpRequestTabs/HttpRequestTabs.tsx), the dot indicator should only appear when scripts are present (count > 0). When no scripts exist, the dot should not be rendered. This is the intended behavior per issue #3731.
📚 Learning: 2025-10-22T22:57:45.959Z
Learnt from: mustafa-sayyed
Repo: requestly/requestly PR: 3723
File: app/src/features/apiClient/screens/apiClient/components/views/http/components/HttpRequestTabs/HttpRequestTabs.tsx:146-151
Timestamp: 2025-10-22T22:57:45.959Z
Learning: In the API Client Scripts tab (app/src/features/apiClient/screens/apiClient/components/views/http/components/HttpRequestTabs/HttpRequestTabs.tsx), the dot indicator should only appear when scripts are present (count > 0). When no scripts exist, the dot should not be rendered. This is the intended behavior per issue #3731.
Applied to files:
app/src/features/apiClient/screens/apiClient/components/views/components/Scripts/components/ScriptEditor/scriptEditor.scssapp/src/features/apiClient/screens/apiClient/components/views/components/request/components/ApiClientRequestTabs/components/RequestTabLabel/RequestTabLabel.tsxapp/src/features/apiClient/screens/apiClient/components/views/components/Scripts/components/ScriptEditor/ScriptEditor.tsx
📚 Learning: 2025-10-27T14:31:06.274Z
Learnt from: Aarushsr12
Repo: requestly/requestly PR: 3719
File: app/src/features/apiClient/screens/apiClient/components/views/components/Collection/components/CollectionRunnerView/components/RunConfigView/RunConfigSettings/runConfigSettings.scss:85-113
Timestamp: 2025-10-27T14:31:06.274Z
Learning: In the requestly/requestly codebase, do not suggest adding fallback values to CSS/SCSS variable usage (e.g., var(--requestly-color-error-darker)). The team relies on their design token system to ensure all tokens are defined, and fallbacks are not needed.
Applied to files:
app/src/features/apiClient/screens/apiClient/components/views/components/Scripts/components/ScriptEditor/scriptEditor.scss
📚 Learning: 2025-10-10T11:08:23.369Z
Learnt from: nafees87n
Repo: requestly/requestly PR: 3655
File: app/src/features/apiClient/screens/apiClient/components/views/components/Collection/components/CollectionRunnerView/components/RunResultView/RunResultContainer/RunResultContainer.tsx:426-426
Timestamp: 2025-10-10T11:08:23.369Z
Learning: In the file `app/src/features/apiClient/screens/apiClient/components/views/components/Collection/components/CollectionRunnerView/components/RunResultView/RunResultContainer/RunResultContainer.tsx`, the `destroyInactiveTabPane={true}` setting is intentional. The loss of state (expandedKeys, scroll position) when switching tabs is acceptable for now and will be addressed in future work.
Applied to files:
app/src/features/apiClient/screens/apiClient/components/views/components/request/components/ApiClientRequestTabs/components/RequestTabLabel/RequestTabLabel.tsxapp/src/features/apiClient/screens/apiClient/components/views/components/request/components/KeyValueTable/KeyValueTable.tsx
📚 Learning: 2025-10-27T16:15:11.603Z
Learnt from: Aarushsr12
Repo: requestly/requestly PR: 3719
File: app/src/features/apiClient/screens/apiClient/components/views/components/Collection/components/CollectionRunnerView/components/RunConfigView/ParseFileModal/ParsedTableView.tsx:33-33
Timestamp: 2025-10-27T16:15:11.603Z
Learning: In ParsedTableView.tsx for the collection runner data file preview table, column titles should preserve the original case from the data file (using `k.charAt(0) + k.slice(1)` or just `k`) rather than capitalizing the first letter. This is because column names are used as variables in request bodies and need exact case matching for variable resolution to work correctly.
Applied to files:
app/src/features/apiClient/screens/apiClient/components/views/components/request/components/KeyValueTable/KeyValueTable.tsx
📚 Learning: 2025-11-25T09:03:46.345Z
Learnt from: CR
Repo: requestly/requestly PR: 0
File: .cursorrules:0-0
Timestamp: 2025-11-25T09:03:46.345Z
Learning: Use TypeScript where applicable in the Requestly project
Applied to files:
app/src/features/apiClient/screens/apiClient/components/views/components/Scripts/components/ScriptEditor/ScriptEditor.tsx
📚 Learning: 2025-09-05T06:46:25.395Z
Learnt from: nafees87n
Repo: requestly/requestly PR: 3504
File: app/src/features/apiClient/helpers/httpRequestExecutor/httpRequestExecutor.ts:149-151
Timestamp: 2025-09-05T06:46:25.395Z
Learning: In the RQAPI types, ScriptType enum values are strings that match property names: ScriptType.PRE_REQUEST = "preRequest" and ScriptType.POST_RESPONSE = "postResponse". This means DEFAULT_SCRIPT_VALUES.preRequest and DEFAULT_SCRIPT_VALUES[RQAPI.ScriptType.PRE_REQUEST] refer to the same value.
Applied to files:
app/src/features/apiClient/screens/apiClient/components/views/components/Scripts/components/ScriptEditor/ScriptEditor.tsx
🧬 Code graph analysis (2)
app/src/features/apiClient/screens/apiClient/components/views/components/request/components/KeyValueTable/KeyValueTable.tsx (2)
app/src/features/apiClient/types.ts (1)
KeyValuePair(37-43)shared/src/types/entities/apiClient/api.ts (1)
KeyValuePair(208-214)
app/src/features/apiClient/screens/apiClient/components/views/components/Scripts/components/ScriptEditor/ScriptEditor.tsx (2)
app/src/features/apiClient/screens/apiClient/components/views/components/request/components/ApiClientRequestTabs/components/RequestTabLabel/RequestTabLabel.tsx (1)
RequestTabLabelIndicator(20-33)app/src/features/apiClient/constants.ts (1)
DEFAULT_SCRIPT_VALUES(68-73)
🔇 Additional comments (7)
app/src/features/apiClient/screens/apiClient/components/views/components/request/components/KeyValueTable/KeyValueTable.tsx (1)
27-35: LGTM for the primary use case.The function correctly creates enabled pairs for user-initiated row additions (via
handleAddPair). The defaultisEnabled: trueis appropriate when users explicitly click "Add More."app/src/features/apiClient/screens/apiClient/components/views/components/Scripts/components/ScriptEditor/scriptEditor.scss (1)
53-60: LGTM! Clean indicator styling.The CSS class definition is clear and appropriate for the green dot indicator. The styling aligns well with the PR objectives.
app/src/features/apiClient/screens/apiClient/components/views/components/request/components/ApiClientRequestTabs/components/RequestTabLabel/RequestTabLabel.tsx (1)
20-29: LGTM! Good component reusability.Exporting
RequestTabLabelIndicatorenables reuse in the ScriptEditor component. The formatting changes are stylistic and maintain the existing logic.app/src/features/apiClient/screens/apiClient/components/views/components/Scripts/components/ScriptEditor/ScriptEditor.tsx (4)
10-10: LGTM! Correct import path.The import aligns with the newly exported
RequestTabLabelIndicatorcomponent.
12-18: LGTM! Good controlled component pattern.The expanded props enable better external control of the component. The
onScriptsChangecallback follows React best practices for controlled components.
68-68: LGTM! Correct dependency management.Adding
scriptsto the dependency array ensures the indicator counts update when script content changes.
75-75: LGTM! Proper immutable update pattern.The callback correctly creates a new scripts object without mutating the original, following React best practices.
...lient/screens/apiClient/components/views/http/components/HttpRequestTabs/HttpRequestTabs.tsx
Outdated
Show resolved
Hide resolved
nafees87n
left a comment
nafees87n
left a comment
There was a problem hiding this comment.
Looks good. Thanks @mustafa-sayyed
There was a problem hiding this comment.
Pull request overview
Fixes the green dot indicator logic for the Params and Body tabs in the API Client, ensuring indicators only appear when actual data is present. The PR also addresses the default enabled state of empty rows in parameter tables.
Key changes:
- Modified Body tab to check actual body content length instead of just presence
- Changed initial empty rows in KeyValueTable to be disabled by default
- Exported RequestTabLabelIndicator component for reuse in script tabs
- Added visual indicators to Pre-request and Post-response script tabs
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| HttpRequestTabs.tsx | Updated Body tab count logic to check body length and reorganized dependency array |
| KeyValueTable.tsx | Modified initial empty row creation to set isEnabled to false by default |
| RequestTabLabel.tsx | Exported RequestTabLabelIndicator component and reformatted code |
| scriptEditor.scss | Added CSS styling for success dot indicator |
| ScriptEditor.tsx | Integrated RequestTabLabelIndicator to show green dots for modified scripts |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
...lient/screens/apiClient/components/views/http/components/HttpRequestTabs/HttpRequestTabs.tsx
Show resolved
Hide resolved
...reens/apiClient/components/views/components/Scripts/components/ScriptEditor/ScriptEditor.tsx
Outdated
Show resolved
Hide resolved
...reens/apiClient/components/views/components/Scripts/components/ScriptEditor/ScriptEditor.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
app/src/features/apiClient/screens/apiClient/components/views/components/Scripts/components/ScriptEditor/ScriptEditor.tsx (1)
10-56: Script tab dot indicators: behavior looks correct; consider DRY-ing the count logicImporting
RequestTabLabelIndicatorand gatingcountonscripts?.preRequest/postResponse !== DEFAULT_SCRIPT_VALUES.*ensures the green dot only appears when there is a “real” script and not just the default template, which matches the intended behavior from issue #3731. Based on learnings, this aligns with the “dot only when scripts present” requirement.To reduce duplication and keep access consistent with
RQAPI.ScriptType, you could extract a small helper:export const ScriptEditor: React.FC<ScriptEditorProps> = ({ scripts, onScriptsChange, focusPostResponse }) => { + const hasCustomScript = (type: RQAPI.ScriptType) => { + const value = scripts?.[type]; + return !!value && value !== DEFAULT_SCRIPT_VALUES[type]; + }; + const activeScriptType = scripts?.[RQAPI.ScriptType.PRE_REQUEST] @@ <Radio.Button className="api-client-script-type-selector__btn" value={RQAPI.ScriptType.PRE_REQUEST}> Pre-request <RequestTabLabelIndicator - count={scripts?.preRequest && scripts?.preRequest !== DEFAULT_SCRIPT_VALUES.preRequest ? 1 : 0} - showDot={true} + count={hasCustomScript(RQAPI.ScriptType.PRE_REQUEST) ? 1 : 0} + showDot /> </Radio.Button> <Radio.Button className="api-client-script-type-selector__btn" value={RQAPI.ScriptType.POST_RESPONSE}> Post-response <RequestTabLabelIndicator - count={scripts?.postResponse && scripts.postResponse !== DEFAULT_SCRIPT_VALUES.postResponse ? 1 : 0} - showDot={true} + count={hasCustomScript(RQAPI.ScriptType.POST_RESPONSE) ? 1 : 0} + showDot /> </Radio.Button>This keeps the “non-default script => dot” rule in one place and avoids repeating magic property names.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/src/features/apiClient/screens/apiClient/components/views/components/Scripts/components/ScriptEditor/ScriptEditor.tsx(3 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: mustafa-sayyed
Repo: requestly/requestly PR: 3723
File: app/src/features/apiClient/screens/apiClient/components/views/http/components/HttpRequestTabs/HttpRequestTabs.tsx:146-151
Timestamp: 2025-10-22T22:57:45.959Z
Learning: In the API Client Scripts tab (app/src/features/apiClient/screens/apiClient/components/views/http/components/HttpRequestTabs/HttpRequestTabs.tsx), the dot indicator should only appear when scripts are present (count > 0). When no scripts exist, the dot should not be rendered. This is the intended behavior per issue #3731.
Learnt from: RuntimeTerror10
Repo: requestly/requestly PR: 3565
File: app/src/features/apiClient/screens/apiClient/components/views/http/components/PathVariableTable/PathVariableTable.tsx:77-79
Timestamp: 2025-09-24T10:55:40.434Z
Learning: In the Requestly API client, the PathVariableTable component is designed to conditionally render - it should only appear when there are path variables in the URL (variables.length > 0), returning null otherwise. This provides a cleaner UX by hiding the entire table section when no path variables are present, rather than showing an empty table state.
📚 Learning: 2025-10-22T22:57:45.959Z
Learnt from: mustafa-sayyed
Repo: requestly/requestly PR: 3723
File: app/src/features/apiClient/screens/apiClient/components/views/http/components/HttpRequestTabs/HttpRequestTabs.tsx:146-151
Timestamp: 2025-10-22T22:57:45.959Z
Learning: In the API Client Scripts tab (app/src/features/apiClient/screens/apiClient/components/views/http/components/HttpRequestTabs/HttpRequestTabs.tsx), the dot indicator should only appear when scripts are present (count > 0). When no scripts exist, the dot should not be rendered. This is the intended behavior per issue #3731.
Applied to files:
app/src/features/apiClient/screens/apiClient/components/views/components/Scripts/components/ScriptEditor/ScriptEditor.tsx
📚 Learning: 2025-11-25T09:03:46.345Z
Learnt from: CR
Repo: requestly/requestly PR: 0
File: .cursorrules:0-0
Timestamp: 2025-11-25T09:03:46.345Z
Learning: Use TypeScript where applicable in the Requestly project
Applied to files:
app/src/features/apiClient/screens/apiClient/components/views/components/Scripts/components/ScriptEditor/ScriptEditor.tsx
📚 Learning: 2025-09-05T06:46:25.395Z
Learnt from: nafees87n
Repo: requestly/requestly PR: 3504
File: app/src/features/apiClient/helpers/httpRequestExecutor/httpRequestExecutor.ts:149-151
Timestamp: 2025-09-05T06:46:25.395Z
Learning: In the RQAPI types, ScriptType enum values are strings that match property names: ScriptType.PRE_REQUEST = "preRequest" and ScriptType.POST_RESPONSE = "postResponse". This means DEFAULT_SCRIPT_VALUES.preRequest and DEFAULT_SCRIPT_VALUES[RQAPI.ScriptType.PRE_REQUEST] refer to the same value.
Applied to files:
app/src/features/apiClient/screens/apiClient/components/views/components/Scripts/components/ScriptEditor/ScriptEditor.tsx
🧬 Code graph analysis (1)
app/src/features/apiClient/screens/apiClient/components/views/components/Scripts/components/ScriptEditor/ScriptEditor.tsx (2)
app/src/features/apiClient/screens/apiClient/components/views/components/request/components/ApiClientRequestTabs/components/RequestTabLabel/RequestTabLabel.tsx (1)
RequestTabLabelIndicator(20-33)app/src/features/apiClient/constants.ts (1)
DEFAULT_SCRIPT_VALUES(68-73)
nafees87n
changed the title
Closes issue: #3731
📜 Summary of changes:
🎥 Demo Video:
Screen.Recording.2025-10-26.013011.mp4
✅ Checklist:
🧪 Test instructions:
Summary by CodeRabbit
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.