feat: modal to UI pages #719
Conversation
RohitR311
added
Type: Enhancement
WalkthroughThis change refactors the robot configuration and integration UI from modal-based components to dedicated page components. It introduces several new page components for robot settings, editing, duplication, scheduling, and integrations, and updates the main Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant RecordingsPage
participant Router
participant ConfigPage
participant API
User->>RecordingsPage: Navigates to robot configuration/integration
RecordingsPage->>Router: Matches config/integration route
Router->>ConfigPage: Renders appropriate page component
ConfigPage->>API: Fetches robot data/settings
API-->>ConfigPage: Returns data
User->>ConfigPage: Interacts (edit, save, duplicate, schedule, integrate)
ConfigPage->>API: Sends updates (if applicable)
API-->>ConfigPage: Confirms changes
ConfigPage->>Router: Navigates back or to next step
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (9)
✅ Files skipped from review due to trivial changes (4)
🚧 Files skipped from review as they are similar to previous changes (3)
🔇 Additional comments (2)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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: 16
♻️ Duplicate comments (3)
src/components/robot/pages/RobotDuplicatePage.tsx (1)
19-63: Use shared type definitions.These interfaces are duplicated across multiple files.
src/components/robot/pages/RobotSettingsPage.tsx (2)
12-56: Use shared type definitions.These interfaces are duplicated across multiple files.
74-85: Improve error handling specificity.Same error handling issue as in other robot page components.
🧹 Nitpick comments (6)
src/components/robot/pages/RobotEditPage.tsx (1)
369-408: Simplify nested array bounds checking.The function has good defensive programming but the deeply nested conditions reduce readability.
const handleLimitChange = ( pairIndex: number, actionIndex: number, argIndex: number, newLimit: number ) => { setRobot((prev) => { if (!prev) return prev; const updatedWorkflow = [...prev.recording.workflow]; const pair = updatedWorkflow[pairIndex]; if (!pair?.what?.[actionIndex]?.args?.[argIndex]) { return prev; } pair.what[actionIndex].args[argIndex].limit = newLimit; setScrapeListLimits((prevLimits) => prevLimits.map((item) => (item.pairIndex === pairIndex && item.actionIndex === actionIndex && item.argIndex === argIndex) ? { ...item, currentLimit: newLimit } : item ) ); return { ...prev, recording: { ...prev.recording, workflow: updatedWorkflow }, }; }); };src/components/robot/pages/RobotConfigPage.tsx (1)
51-60: Make the layout responsive instead of fixed width.The component uses a fixed width of 1000px which can cause issues on smaller screens.
sx={{ maxWidth: 1000, margin: 'auto', px: 4, py: 3, minHeight: '80vh', display: 'flex', flexDirection: 'column', width: '100%', // Remove fixed width // Add responsive padding px: { xs: 2, sm: 3, md: 4 }, }}src/components/robot/pages/ScheduleSettingsPage.tsx (1)
71-96: Improve error handling and logging.The error handling doesn't log the actual error, making debugging difficult.
} catch (error) { console.error("Failed to delete schedule:", error); notify("error", t("schedule_settings.errors.delete_failed")); }src/components/robot/Recordings.tsx (1)
5-8: Remove unused importScheduleSettingsThe
ScheduleSettingstype is imported but only used in the component's props interface. Consider importing it where it's actually used to reduce coupling.-import { - ScheduleSettings, - ScheduleSettingsPage, -} from "./pages/ScheduleSettingsPage"; +import { ScheduleSettingsPage } from "./pages/ScheduleSettingsPage";Then import
ScheduleSettingsfrom the appropriate location where it's defined and used.src/components/robot/pages/RobotIntegrationPage.tsx (2)
491-497: Use theme variables instead of hard-coded colors and stylesThe component uses hard-coded colors and inline styles which reduces maintainability and consistency. Consider using Material-UI theme variables.
-<Alert severity="info" sx={{ marginTop: "10px", border: "1px solid #ff00c3" }}> +<Alert severity="info" sx={{ marginTop: 1.25, border: (theme) => `1px solid ${theme.palette.primary.main}` }}> -style={{ marginLeft: "4px", fontWeight: "bold" }} +sx={{ ml: 0.5, fontWeight: 'bold' }} -style={{ display: "flex", flexDirection: "column", alignItems: "center", background: 'white', color: '#ff00c3' }} +sx={{ + display: "flex", + flexDirection: "column", + alignItems: "center", + bgcolor: 'background.paper', + color: 'primary.main' +}}Also applies to: 549-554, 693-696, 703-706, 713-716
769-773: Improve accessibility with proper ARIA labelsThe icon buttons in the webhook table lack proper accessibility labels for screen readers.
-<IconButton size="small" onClick={() => testWebhookSetting(webhook.id)} disabled={loading || !webhook.active} title="Test"><ScienceIcon fontSize="small" /></IconButton> -<IconButton size="small" onClick={() => editWebhookSetting(webhook)} disabled={loading} title="Edit"><EditIcon fontSize="small" /></IconButton> -<IconButton size="small" onClick={() => deleteWebhookSetting(webhook.id)} disabled={loading} title="Delete"><DeleteIcon fontSize="small" /></IconButton> +<IconButton size="small" onClick={() => testWebhookSetting(webhook.id)} disabled={loading || !webhook.active} aria-label="Test webhook"><ScienceIcon fontSize="small" /></IconButton> +<IconButton size="small" onClick={() => editWebhookSetting(webhook)} disabled={loading} aria-label="Edit webhook"><EditIcon fontSize="small" /></IconButton> +<IconButton size="small" onClick={() => deleteWebhookSetting(webhook.id)} disabled={loading} aria-label="Delete webhook"><DeleteIcon fontSize="small" /></IconButton>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/components/robot/Recordings.tsx(4 hunks)src/components/robot/pages/RobotConfigPage.tsx(1 hunks)src/components/robot/pages/RobotDuplicatePage.tsx(1 hunks)src/components/robot/pages/RobotEditPage.tsx(1 hunks)src/components/robot/pages/RobotIntegrationPage.tsx(1 hunks)src/components/robot/pages/RobotSettingsPage.tsx(1 hunks)src/components/robot/pages/ScheduleSettingsPage.tsx(1 hunks)
🧰 Additional context used
🪛 Biome (2.1.2)
src/components/robot/pages/RobotDuplicatePage.tsx
[error] 167-167: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
🪛 ast-grep (0.38.6)
src/components/robot/pages/RobotDuplicatePage.tsx
[warning] 166-166: Usage of dangerouslySetInnerHTML detected. This bypasses React's built-in XSS protection. Always sanitize HTML content using libraries like DOMPurify before injecting it into the DOM to prevent XSS attacks.
Context: dangerouslySetInnerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://reactjs.org/docs/dom-elements.html#dangerouslysetinnerhtml
- https://cwe.mitre.org/data/definitions/79.html
(react-unsafe-html-injection)
| color: '#ff00c3', | ||
| borderColor: '#ff00c3', | ||
| bgcolor: 'whitesmoke', | ||
| '&:hover': { | ||
| bgcolor: '#f0f0f0', | ||
| borderColor: '#cc0099' | ||
| } | ||
| }} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use theme colors instead of hardcoded values.
The component uses hardcoded colors (#ff00c3, #cc0099, whitesmoke) instead of theme colors, making it difficult to maintain consistency and support theming.
// Add to theme or use theme palette
const theme = useTheme();
// Define custom colors in theme
const primaryBrand = theme.palette.primary.main || '#ff00c3';
const primaryBrandDark = theme.palette.primary.dark || '#cc0099';
// Use in styles
sx={{
color: primaryBrand,
borderColor: primaryBrand,
bgcolor: theme.palette.grey[100], // instead of 'whitesmoke'
'&:hover': {
bgcolor: theme.palette.grey[200],
borderColor: primaryBrandDark
}
}}Also applies to: 147-152, 168-171
🤖 Prompt for AI Agents
In src/components/robot/pages/RobotConfigPage.tsx around lines 127 to 134,
replace the hardcoded color values (#ff00c3, #cc0099, whitesmoke) with colors
from the theme palette to ensure consistency and support theming. Use the
useTheme hook to access the theme, then define variables for primaryBrand and
primaryBrandDark from theme.palette.primary.main and theme.palette.primary.dark
respectively, and replace 'whitesmoke' with theme.palette.grey[100]. Apply these
theme colors in the sx style object for color, borderColor, and bgcolor,
including hover styles. Repeat this approach for lines 147-152 and 168-171 as
well.
| const getRobot = async () => { | ||
| if (recordingId) { | ||
| try { | ||
| const robot = await getStoredRecording(recordingId); | ||
| setRobot(robot); | ||
| } catch (error) { | ||
| notify("error", t("robot_duplication.notifications.robot_not_found")); | ||
| } | ||
| } else { | ||
| notify("error", t("robot_duplication.notifications.robot_not_found")); | ||
| } | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Distinguish between different error scenarios.
Like in RobotEditPage, the error handling shows the same message for different scenarios.
const getRobot = async () => {
if (!recordingId) {
notify("error", t("robot_duplication.notifications.no_recording_id"));
return;
}
try {
const robot = await getStoredRecording(recordingId);
setRobot(robot);
} catch (error) {
console.error("Failed to fetch robot:", error);
notify("error", t("robot_duplication.notifications.fetch_failed"));
}
};🤖 Prompt for AI Agents
In src/components/robot/pages/RobotDuplicatePage.tsx around lines 98 to 109, the
error handling currently uses the same notification message for missing
recordingId and fetch failures. Refactor the getRobot function to first check if
recordingId is missing and notify with a specific "no_recording_id" message,
then in the catch block log the error to the console and notify with a distinct
"fetch_failed" message to clearly distinguish the error scenarios.
| <span | ||
| dangerouslySetInnerHTML={{ | ||
| __html: t("robot_duplication.descriptions.example", { | ||
| url1: "<code>producthunt.com/topics/api</code>", | ||
| url2: "<code>producthunt.com/topics/database</code>", | ||
| }), | ||
| }} | ||
| /> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Replace dangerouslySetInnerHTML with safe React components.
Using dangerouslySetInnerHTML poses XSS risks and bypasses React's built-in protections. Even though the content comes from translations, it's safer to use React components.
// Instead of dangerouslySetInnerHTML, create a safe component:
const ExampleDescription = () => {
const { t } = useTranslation();
return (
<span>
{t("robot_duplication.descriptions.example_prefix")}
<code>producthunt.com/topics/api</code>
{t("robot_duplication.descriptions.example_middle")}
<code>producthunt.com/topics/database</code>
{t("robot_duplication.descriptions.example_suffix")}
</span>
);
};
// Then use it in the render:
<ExampleDescription />Alternatively, update your translation file to use placeholders and render safely:
<span>
{t("robot_duplication.descriptions.example", {
interpolation: { escapeValue: false },
components: {
code1: <code>producthunt.com/topics/api</code>,
code2: <code>producthunt.com/topics/database</code>
}
})}
</span>🧰 Tools
🪛 Biome (2.1.2)
[error] 167-167: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
🪛 ast-grep (0.38.6)
[warning] 166-166: Usage of dangerouslySetInnerHTML detected. This bypasses React's built-in XSS protection. Always sanitize HTML content using libraries like DOMPurify before injecting it into the DOM to prevent XSS attacks.
Context: dangerouslySetInnerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://reactjs.org/docs/dom-elements.html#dangerouslysetinnerhtml
- https://cwe.mitre.org/data/definitions/79.html
(react-unsafe-html-injection)
🤖 Prompt for AI Agents
In src/components/robot/pages/RobotDuplicatePage.tsx around lines 166 to 173,
replace the usage of dangerouslySetInnerHTML with safe React components to avoid
XSS risks. Refactor the code to either create a dedicated React component that
assembles the translated text and code elements separately or update the
translation to use placeholders and render the code elements as React components
within the translation call. This ensures the content is safely rendered without
bypassing React's protections.
| interface RobotMeta { | ||
| name: string; | ||
| id: string; | ||
| createdAt: string; | ||
| pairs: number; | ||
| updatedAt: string; | ||
| params: any[]; | ||
| url?: string; | ||
| } | ||
|
|
||
| interface RobotWorkflow { | ||
| workflow: WhereWhatPair[]; | ||
| } | ||
|
|
||
| interface ScheduleConfig { | ||
| runEvery: number; | ||
| runEveryUnit: "MINUTES" | "HOURS" | "DAYS" | "WEEKS" | "MONTHS"; | ||
| startFrom: | ||
| | "SUNDAY" | ||
| | "MONDAY" | ||
| | "TUESDAY" | ||
| | "WEDNESDAY" | ||
| | "THURSDAY" | ||
| | "FRIDAY" | ||
| | "SATURDAY"; | ||
| atTimeStart?: string; | ||
| atTimeEnd?: string; | ||
| timezone: string; | ||
| lastRunAt?: Date; | ||
| nextRunAt?: Date; | ||
| cronExpression?: string; | ||
| } | ||
|
|
||
| export interface RobotSettings { | ||
| id: string; | ||
| userId?: number; | ||
| recording_meta: RobotMeta; | ||
| recording: RobotWorkflow; | ||
| google_sheet_email?: string | null; | ||
| google_sheet_name?: string | null; | ||
| google_sheet_id?: string | null; | ||
| google_access_token?: string | null; | ||
| google_refresh_token?: string | null; | ||
| schedule?: ScheduleConfig | null; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Extract shared interfaces to avoid duplication.
These interfaces (RobotMeta, RobotWorkflow, ScheduleConfig, RobotSettings) are duplicated across multiple files (RobotEditPage, RobotDuplicatePage, RobotSettingsPage). This violates the DRY principle and makes maintenance difficult.
Create a shared types file:
// src/components/robot/types/robot.types.ts
export interface RobotMeta {
name: string;
id: string;
createdAt: string;
pairs: number;
updatedAt: string;
params: any[];
url?: string;
}
export interface RobotWorkflow {
workflow: WhereWhatPair[];
}
export interface ScheduleConfig {
runEvery: number;
runEveryUnit: "MINUTES" | "HOURS" | "DAYS" | "WEEKS" | "MONTHS";
startFrom: "SUNDAY" | "MONDAY" | "TUESDAY" | "WEDNESDAY" | "THURSDAY" | "FRIDAY" | "SATURDAY";
atTimeStart?: string;
atTimeEnd?: string;
timezone: string;
lastRunAt?: Date;
nextRunAt?: Date;
cronExpression?: string;
}
export interface RobotSettings {
id: string;
userId?: number;
recording_meta: RobotMeta;
recording: RobotWorkflow;
google_sheet_email?: string | null;
google_sheet_name?: string | null;
google_sheet_id?: string | null;
google_access_token?: string | null;
google_refresh_token?: string | null;
schedule?: ScheduleConfig | null;
}Then import from this shared file in each component.
🤖 Prompt for AI Agents
In src/components/robot/pages/RobotEditPage.tsx lines 18 to 62, the interfaces
RobotMeta, RobotWorkflow, ScheduleConfig, and RobotSettings are duplicated and
should be extracted to a shared types file to follow the DRY principle. Create a
new file src/components/robot/types/robot.types.ts and move these interface
definitions there. Then, remove these interfaces from RobotEditPage.tsx and
import them from the new shared types file. Repeat this import update in other
files where these interfaces are duplicated.
| function extractInitialCredentials(workflow: any[]): Credentials { | ||
| const credentials: Credentials = {}; | ||
|
|
||
| const isPrintableCharacter = (char: string): boolean => { | ||
| return char.length === 1 && !!char.match(/^[\x20-\x7E]$/); | ||
| }; | ||
|
|
||
| workflow.forEach((step) => { | ||
| if (!step.what) return; | ||
|
|
||
| let currentSelector = ""; | ||
| let currentValue = ""; | ||
| let currentType = ""; | ||
| let i = 0; | ||
|
|
||
| while (i < step.what.length) { | ||
| const action = step.what[i]; | ||
|
|
||
| if (!action.action || !action.args?.[0]) { | ||
| i++; | ||
| continue; | ||
| } | ||
|
|
||
| const selector = action.args[0]; | ||
|
|
||
| if ( | ||
| action.action === "type" && | ||
| action.args?.length >= 2 && | ||
| typeof action.args[1] === "string" && | ||
| action.args[1].length > 1 | ||
| ) { | ||
| if (!credentials[selector]) { | ||
| credentials[selector] = { | ||
| value: action.args[1], | ||
| type: action.args[2] || "text", | ||
| }; | ||
| } | ||
| i++; | ||
| continue; | ||
| } | ||
|
|
||
| if ( | ||
| (action.action === "type" || action.action === "press") && | ||
| action.args?.length >= 2 && | ||
| typeof action.args[1] === "string" | ||
| ) { | ||
| if (selector !== currentSelector) { | ||
| if (currentSelector && currentValue) { | ||
| credentials[currentSelector] = { | ||
| value: currentValue, | ||
| type: currentType || "text", | ||
| }; | ||
| } | ||
| currentSelector = selector; | ||
| currentValue = credentials[selector]?.value || ""; | ||
| currentType = | ||
| action.args[2] || credentials[selector]?.type || "text"; | ||
| } | ||
|
|
||
| const character = action.args[1]; | ||
|
|
||
| if (isPrintableCharacter(character)) { | ||
| currentValue += character; | ||
| } else if (character === "Backspace") { | ||
| currentValue = currentValue.slice(0, -1); | ||
| } | ||
|
|
||
| if (!currentType && action.args[2]?.toLowerCase() === "password") { | ||
| currentType = "password"; | ||
| } | ||
|
|
||
| let j = i + 1; | ||
| while (j < step.what.length) { | ||
| const nextAction = step.what[j]; | ||
| if ( | ||
| !nextAction.action || | ||
| !nextAction.args?.[0] || | ||
| nextAction.args[0] !== selector || | ||
| (nextAction.action !== "type" && nextAction.action !== "press") | ||
| ) { | ||
| break; | ||
| } | ||
| if (nextAction.args[1] === "Backspace") { | ||
| currentValue = currentValue.slice(0, -1); | ||
| } else if (isPrintableCharacter(nextAction.args[1])) { | ||
| currentValue += nextAction.args[1]; | ||
| } | ||
| j++; | ||
| } | ||
|
|
||
| credentials[currentSelector] = { | ||
| value: currentValue, | ||
| type: currentType, | ||
| }; | ||
|
|
||
| i = j; | ||
| } else { | ||
| i++; | ||
| } | ||
| } | ||
|
|
||
| if (currentSelector && currentValue) { | ||
| credentials[currentSelector] = { | ||
| value: currentValue, | ||
| type: currentType || "text", | ||
| }; | ||
| } | ||
| }); | ||
|
|
||
| return credentials; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Refactor complex credential extraction logic.
The extractInitialCredentials function is 110 lines long with deeply nested loops and complex control flow. This makes it difficult to understand, test, and maintain.
Break down the function into smaller, focused functions:
function extractInitialCredentials(workflow: any[]): Credentials {
const credentials: Credentials = {};
workflow.forEach((step) => {
if (!step.what) return;
processWorkflowStep(step.what, credentials);
});
return credentials;
}
function processWorkflowStep(actions: any[], credentials: Credentials) {
let context = createProcessingContext();
for (let i = 0; i < actions.length;) {
const action = actions[i];
if (shouldSkipAction(action)) {
i++;
continue;
}
if (isSimpleTypeAction(action)) {
handleSimpleTypeAction(action, credentials);
i++;
} else if (isCharacterSequenceAction(action)) {
i = processCharacterSequence(actions, i, credentials, context);
} else {
i++;
}
}
finalizeContext(context, credentials);
}
// Additional helper functions for isPrintableCharacter, processCharacterSequence, etc.🤖 Prompt for AI Agents
In src/components/robot/pages/RobotEditPage.tsx around lines 192 to 302, the
extractInitialCredentials function is overly long and complex with nested loops
and conditionals. Refactor by splitting it into smaller helper functions: create
a main extractInitialCredentials that iterates workflow steps and calls a new
processWorkflowStep function to handle each step's actions. Further break down
logic into focused helpers like isPrintableCharacter, shouldSkipAction,
isSimpleTypeAction, handleSimpleTypeAction, processCharacterSequence, and
finalizeContext. This modular approach improves readability, testability, and
maintainability.
| useEffect(() => { | ||
| const fetchUserEmail = async () => { | ||
| if (robot && robot.userId) { | ||
| try { | ||
| const userData = await getUserById(robot.userId.toString()); | ||
| if (userData && userData.user) { | ||
| setUserEmail(userData.user.email); | ||
| } | ||
| } catch (error) { | ||
| console.error("Failed to fetch user email:", error); | ||
| } | ||
| } | ||
| }; | ||
| fetchUserEmail(); | ||
| }, [robot?.userId]); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Notify users when creator information fails to load.
The error is only logged to console without user notification, leading to a silent failure where the creator field appears empty.
useEffect(() => {
const fetchUserEmail = async () => {
if (robot && robot.userId) {
try {
const userData = await getUserById(robot.userId.toString());
if (userData && userData.user) {
setUserEmail(userData.user.email);
}
} catch (error) {
console.error("Failed to fetch user email:", error);
setUserEmail(t("robot_settings.errors.creator_unavailable"));
// Optionally show a notification
notify("warning", t("robot_settings.errors.creator_fetch_failed"));
}
}
};
fetchUserEmail();
}, [robot?.userId]);🤖 Prompt for AI Agents
In src/components/robot/pages/RobotSettingsPage.tsx around lines 100 to 114, the
fetchUserEmail function only logs errors to the console when failing to load the
creator's email, causing silent failures. Modify the catch block to set a
user-friendly error message in the userEmail state and trigger a warning
notification to inform users that the creator information could not be fetched,
using the existing translation keys and notify function.
| useEffect(() => { | ||
| const fetchSchedule = async () => { | ||
| await getRobotSchedule(); | ||
| }; | ||
| fetchSchedule(); | ||
| }, []); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix useEffect dependencies.
The useEffect uses getRobotSchedule without including it in the dependency array, which could lead to stale closures.
useEffect(() => {
const fetchSchedule = async () => {
if (recordingId) {
try {
const scheduleData = await getSchedule(recordingId);
setSchedule(scheduleData);
} catch (error) {
console.error("Failed to fetch robot schedule:", error);
}
}
};
fetchSchedule();
}, [recordingId]); // Add recordingId as dependency🤖 Prompt for AI Agents
In src/components/robot/pages/ScheduleSettingsPage.tsx around lines 111 to 116,
the useEffect hook calls getRobotSchedule but does not include it or relevant
dependencies in the dependency array, risking stale closures. Update the
useEffect to include all necessary dependencies such as recordingId and
getRobotSchedule (or the equivalent function) in the dependency array. Also, add
proper error handling inside the async function and conditionally fetch the
schedule only when recordingId is available.
| {["MONTHS", "WEEKS"].includes(settings.runEveryUnit) | ||
| ? t("schedule_settings.start_from") | ||
| : t("schedule_settings.start_from")} | ||
| : {schedule.startFrom.charAt(0).toUpperCase() + schedule.startFrom.slice(1).toLowerCase()} |
There was a problem hiding this comment.
Remove redundant ternary conditions.
Both branches of the ternary operator return the same translation key, making the condition redundant.
- {["MONTHS", "WEEKS"].includes(settings.runEveryUnit)
- ? t("schedule_settings.start_from")
- : t("schedule_settings.start_from")}
+ {t("schedule_settings.start_from")}If different labels are intended for different units, update the translation keys:
{["MONTHS", "WEEKS"].includes(settings.runEveryUnit)
? t("schedule_settings.start_from_day")
: t("schedule_settings.start_from_time")}Also applies to: 267-270
🤖 Prompt for AI Agents
In src/components/robot/pages/ScheduleSettingsPage.tsx around lines 203 to 206,
the ternary condition is redundant because both branches return the same
translation key. Remove the ternary operator and use a single call to
t("schedule_settings.start_from"). If different labels are intended for
different units, update the keys accordingly as suggested. Also apply the same
fix to lines 267 to 270.
| <RobotIntegrationPage handleStart={() => {}} robotPath={"robots"} /> | ||
| ); | ||
| } else if (currentPath.endsWith("/settings")) { | ||
| return ( | ||
| <RobotSettingsModal | ||
| isOpen={true} | ||
| handleClose={handleClose} | ||
| handleStart={() => {}} | ||
| /> | ||
| ); | ||
| return <RobotSettingsPage handleStart={() => {}} />; | ||
| } else if (currentPath.endsWith("/edit")) { | ||
| return ( | ||
| <RobotEditModal | ||
| isOpen={true} | ||
| handleClose={handleClose} | ||
| handleStart={() => {}} | ||
| /> | ||
| ); | ||
| return <RobotEditPage handleStart={() => {}} />; | ||
| } else if (currentPath.endsWith("/duplicate")) { | ||
| return ( | ||
| <RobotDuplicationModal | ||
| isOpen={true} | ||
| handleClose={handleClose} | ||
| handleStart={() => {}} | ||
| /> | ||
| ); | ||
| return <RobotDuplicatePage handleStart={() => {}} />; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify the necessity of empty handleStart callbacks
Most page components receive empty handleStart callbacks. This suggests either:
- These components don't need this prop and it should be removed
- The implementation is incomplete
Please verify if these callbacks are required and implement them if needed, or remove the prop from the component interfaces.
🏁 Script executed:
#!/bin/bash
# Check if handleStart is used in the page components
echo "Checking RobotIntegrationPage for handleStart usage:"
ast-grep --pattern 'handleStart' src/components/robot/pages/RobotIntegrationPage.tsx
echo -e "\nChecking other page components:"
fd -e tsx . src/components/robot/pages/ --exec ast-grep --pattern 'handleStart' {}Length of output: 613
Remove unused handleStart props from integration & settings pages
Both RobotIntegrationPage and RobotSettingsPage never invoke the handleStart callback (as confirmed by AST grep), so passing an empty no-op adds dead code. You can simplify the routing logic by removing the prop from these pages’ interfaces and from their JSX, while retaining it (and supplying a real handler) only where it’s actually used (RobotEditPage, RobotDuplicatePage, ScheduleSettingsPage).
Locations to update:
src/components/robot/Recordings.tsx– lines renderingRobotIntegrationPage&RobotSettingsPagesrc/components/robot/pages/RobotIntegrationPage.tsx– removehandleStartfrom the props interfacesrc/components/robot/pages/RobotSettingsPage.tsx– removehandleStartfrom the props interface
Suggested diff in Recordings.tsx:
- <RobotIntegrationPage handleStart={() => {}} robotPath="robots" />
+ <RobotIntegrationPage robotPath="robots" />
- } else if (currentPath.endsWith("/settings")) {
- return <RobotSettingsPage handleStart={() => {}} />;
+ } else if (currentPath.endsWith("/settings")) {
+ return <RobotSettingsPage />;And drop handleStart?: … from those pages’ Props definitions.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <RobotIntegrationPage handleStart={() => {}} robotPath={"robots"} /> | |
| ); | |
| } else if (currentPath.endsWith("/settings")) { | |
| return ( | |
| <RobotSettingsModal | |
| isOpen={true} | |
| handleClose={handleClose} | |
| handleStart={() => {}} | |
| /> | |
| ); | |
| return <RobotSettingsPage handleStart={() => {}} />; | |
| } else if (currentPath.endsWith("/edit")) { | |
| return ( | |
| <RobotEditModal | |
| isOpen={true} | |
| handleClose={handleClose} | |
| handleStart={() => {}} | |
| /> | |
| ); | |
| return <RobotEditPage handleStart={() => {}} />; | |
| } else if (currentPath.endsWith("/duplicate")) { | |
| return ( | |
| <RobotDuplicationModal | |
| isOpen={true} | |
| handleClose={handleClose} | |
| handleStart={() => {}} | |
| /> | |
| ); | |
| return <RobotDuplicatePage handleStart={() => {}} />; | |
| <RobotIntegrationPage robotPath="robots" /> | |
| ); | |
| } else if (currentPath.endsWith("/settings")) { | |
| return <RobotSettingsPage />; | |
| } else if (currentPath.endsWith("/edit")) { | |
| return <RobotEditPage handleStart={() => {}} />; | |
| } else if (currentPath.endsWith("/duplicate")) { | |
| return <RobotDuplicatePage handleStart={() => {}} />; |
🤖 Prompt for AI Agents
In src/components/robot/Recordings.tsx around lines 103 to 110, remove the
unused handleStart prop from the JSX elements rendering RobotIntegrationPage and
RobotSettingsPage since these components do not use this prop. Additionally, in
src/components/robot/pages/RobotIntegrationPage.tsx and
src/components/robot/pages/RobotSettingsPage.tsx, remove handleStart from their
props interfaces to clean up dead code and simplify the components' interfaces.
| const currentPath = location.pathname; | ||
| const isConfigPage = | ||
| currentPath.includes("/schedule") || | ||
| currentPath.includes("/integrate") || | ||
| currentPath.includes("/settings") || | ||
| currentPath.includes("/edit") || | ||
| currentPath.includes("/duplicate") || | ||
| currentPath.includes("/run"); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve route pattern matching for better maintainability
The current approach with multiple includes() checks is verbose and error-prone. Consider using a more maintainable pattern.
- const currentPath = location.pathname;
- const isConfigPage =
- currentPath.includes("/schedule") ||
- currentPath.includes("/integrate") ||
- currentPath.includes("/settings") ||
- currentPath.includes("/edit") ||
- currentPath.includes("/duplicate") ||
- currentPath.includes("/run");
+ const currentPath = location.pathname;
+ const configRoutes = ["/schedule", "/integrate", "/settings", "/edit", "/duplicate", "/run"];
+ const isConfigPage = configRoutes.some(route => currentPath.includes(route));This makes it easier to add or remove routes in the future.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const currentPath = location.pathname; | |
| const isConfigPage = | |
| currentPath.includes("/schedule") || | |
| currentPath.includes("/integrate") || | |
| currentPath.includes("/settings") || | |
| currentPath.includes("/edit") || | |
| currentPath.includes("/duplicate") || | |
| currentPath.includes("/run"); | |
| const currentPath = location.pathname; | |
| const configRoutes = ["/schedule", "/integrate", "/settings", "/edit", "/duplicate", "/run"]; | |
| const isConfigPage = configRoutes.some(route => currentPath.includes(route)); |
🤖 Prompt for AI Agents
In src/components/robot/Recordings.tsx around lines 115 to 122, replace the
multiple includes() checks for route matching with a single, maintainable
pattern. Use an array of route segments and check if any of them match the
current path using a method like some() combined with includes(). This will
simplify the code and make it easier to add or remove routes in the future.
2 participants
What this PR does?
Revamps the UI for Robot configuration actions such as schedule, edit, settings, duplicate to have their own page instead of a modal component.
2025-08-04.23-12-24.mp4
Summary by CodeRabbit
New Features
Refactor