Add Route Map #99
Add Route Map #99
Conversation
…te Shuttlecoke_i color
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⛔ Files ignored due to path filters (13)
📒 Files selected for processing (17)
Tip CodeRabbit can approve the review once all CodeRabbit's comments are resolved.Enable the 🪧 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 (
|
Deploying hybus-genesis with
|
| Latest commit: |
8abb935
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://ca6c6b60.hybus-genesis.pages.dev |
| Branch Preview URL: | https://dev-route-map-x150.hybus-genesis.pages.dev |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (13)
src/app/data/shuttle/organizedTimetables.ts (1)
6-7: New shuttle identifier property addedThe addition of the
isShuttleIboolean property to theOrganizedTimetablestype enhances the data structure to support route map functionality. This property will help identify and potentially render specific shuttle types differently.However, the name
isShuttleIis not self-explanatory. Consider using a more descriptive name likeisInterCampusShuttleor adding a comment explaining what "ShuttleI" represents.- isShuttleI: boolean, + isInterCampusShuttle: boolean, // Indicates if this is an inter-campus shuttlesrc/app/reducer/store.ts (2)
5-8: Consider usingPayloadActiontype from Redux ToolkitInstead of creating a custom
Actioninterface, consider using the built-inPayloadActiontype from Redux Toolkit which is designed for this purpose.-interface Action{ - type: string, - payload: SingleShuttleSchedule -} +import { PayloadAction } from "@reduxjs/toolkit";
10-23: Simplify state clearing logicThe while loop to clear the array is inefficient. Since Redux Toolkit uses Immer, you can directly assign an empty array or use a simpler approach.
const actions = createSlice({ name: 'actions', initialState: [] as SingleShuttleSchedule[], reducers: { updateActions: (state, action: Action) => { - while(state.length > 0){ - state.pop() - } + state.length = 0; // Clear the array more efficiently if(action.payload !== undefined){ state.push(action.payload) } } } })Also, since you're only storing a single item, consider using a single object instead of an array:
const actions = createSlice({ name: 'actions', - initialState: [] as SingleShuttleSchedule[], + initialState: null as SingleShuttleSchedule | null, reducers: { - updateActions: (state, action: Action) => { - while(state.length > 0){ - state.pop() - } - if(action.payload !== undefined){ - state.push(action.payload) - } + updateActions: (state, action: PayloadAction<SingleShuttleSchedule | null>) => { + return action.payload; } } })src/app/components/fulltime/FullTime.tsx (4)
126-129: Consider removing commented codeThe commented-out
YesulinMinuteWrapperstyled component is no longer being used. Commented code can confuse developers and adds to maintenance burden. If it's no longer needed, it should be removed.-// const YesulinMinuteWrapper = styled.span<{ $itemCount: number }>` -// ${tw`inline-block text-green-500`} -// ${({ $itemCount }) => ($itemCount === 0 ? tw`hidden` : undefined)} -// `
188-197: Optimize duplicate checking logicThe current implementation checks each direct time against every directY time to filter out duplicates. This approach works but could be inefficient for larger datasets.
Consider using a Set to make this check more efficient:
- {props.direct.map((time, idx) => { - let isExist = false - props.directY.map((ytime) => { - time === ytime ? (isExist = true) : null - }) - return( - isExist ? null : - <React.Fragment key={idx}> - <span>{time} </span> - </React.Fragment> + {props.direct.map((time, idx) => { + const directYSet = new Set(props.directY); + return( + directYSet.has(time) ? null : + <React.Fragment key={idx}> + <span>{time} </span> + </React.Fragment> ) })}
278-281: Fix trailing whitespace and improve readabilityThere's unnecessary trailing whitespace after the closing brace.
- } - if (schedule.type === 'R'){ + } else if (schedule.type === 'R') { single.isShuttleI = true }
317-318: Simplify conditional rendering logicThe current condition is complex and uses double negation. It can be simplified for better readability.
- {schedule.direct.length + schedule.circle.length === 0 && (schedule.directY.length === 0 && schedule.jungang.length === 0) ? null : ( + {schedule.direct.length > 0 || schedule.circle.length > 0 || schedule.directY.length > 0 || schedule.jungang.length > 0 ? ( <TimeBox time={schedule.time} direct={schedule.direct} directY={schedule.directY} circle={schedule.circle} jungang={schedule.jungang} isShuttleI={schedule.isShuttleI} count={countChip} /> - )} + ) : null}src/App.tsx (2)
130-134: Consider simplifying transition logicThe current transition container uses both opacity and display:hidden, which could cause issues since elements with display:none don't animate. Consider using only opacity with pointer-events for a more predictable animation.
const RouteIndexContainer = styled.div<{status: string}>` ${tw`absolute top-0 inset-0 flex place-content-center items-center transition ease-in-out duration-300`} - ${(props) => props.status === 'exited' ? tw`opacity-100` : tw`opacity-0`} - ${(props) => props.status === 'entered' ? tw`hidden`: tw``} + ${(props) => props.status === 'exited' ? tw`opacity-100 pointer-events-auto` : tw`opacity-0 pointer-events-none`} `
448-478: Add comments to explain transition statesWhile the Transition implementation works well, it lacks comments explaining what each state value ('entered', 'exited', etc.) represents. This would make the code more maintainable.
<Transition in={routeCardClick} timeout={150} + {/* States: 'entering', 'entered', 'exiting', 'exited' */} > {(state) => ( <> <RouteIndexCardView status={state} onClick={() => {setRouteCardClick(!routeCardClick)}}>Additionally, consider extracting the onClick handler to a named function for better readability:
+ const toggleRouteCard = () => setRouteCardClick(!routeCardClick); + <Transition in={routeCardClick} timeout={150} > {(state) => ( <> - <RouteIndexCardView status={state} onClick={() => {setRouteCardClick(!routeCardClick)}}> + <RouteIndexCardView status={state} onClick={toggleRouteCard}>.pnp.cjs (1)
3363-3397: Enhanced @reduxjs/toolkit Configuration
A comprehensive configuration for"@reduxjs/toolkit"appears here with two variant entries—one direct (withlinkTypeset toSOFT) and one virtual (withlinkTypeset toHARD). Please double-check that the differentiation in link types is intentional, and that the accompanying peer dependencies (such as"@types/react"and"react-redux") are consistent with your project’s requirements.src/app/components/routemap/RouteMap.tsx (3)
18-35: Consider consolidating repeated styled components into a single component with dynamic color props.We've defined multiple components (CycleCircle, DirectCircle, YesulinCircle, JungangCircle, EndCircle) that differ only by color and margin. We can reduce duplication by reusing one base component with a styled prop.
91-165: Reduce repetitive logic indirectInput.The loops for building route station elements contain conditional logic with partially duplicated structures, making maintenance and readability more difficult. Consider extracting repeated code into helper functions or data-driven approaches for generating route stations.
297-332: Streamline route highlight logic in the final effect.The branching logic based on different
props.tabvalues (lines 308-324) might be simplified or combined with a data-driven approach. This can reduce code repetition and make it easier to add new route types without duplicating code blocks.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (13)
.yarn/cache/@babel-runtime-npm-7.26.0-9afa3c4ef6-9f4ea1c1d5.zipis excluded by!**/.yarn/**,!**/*.zip.yarn/cache/@reduxjs-toolkit-npm-2.5.0-9bdc99574d-b24ee7e89b.zipis excluded by!**/.yarn/**,!**/*.zip.yarn/cache/@types-react-transition-group-npm-4.4.11-dda183055d-a7f4de6e5f.zipis excluded by!**/.yarn/**,!**/*.zip.yarn/cache/@types-use-sync-external-store-npm-0.0.6-9e5c635381-a95ce33066.zipis excluded by!**/.yarn/**,!**/*.zip.yarn/cache/dom-helpers-npm-5.2.1-b38bb4470b-bed2341adf.zipis excluded by!**/.yarn/**,!**/*.zip.yarn/cache/immer-npm-10.1.1-973ae10d09-9dacf1e8c2.zipis excluded by!**/.yarn/**,!**/*.zip.yarn/cache/react-redux-npm-9.2.0-d87bb27c82-b3d2f89f46.zipis excluded by!**/.yarn/**,!**/*.zip.yarn/cache/react-transition-group-npm-4.4.5-98ea4ef96e-ca32d3fd21.zipis excluded by!**/.yarn/**,!**/*.zip.yarn/cache/redux-npm-5.0.1-f8e6b1cb23-a373f9ed65.zipis excluded by!**/.yarn/**,!**/*.zip.yarn/cache/redux-thunk-npm-3.1.0-6a8fdd3211-38c563db5f.zipis excluded by!**/.yarn/**,!**/*.zip.yarn/cache/reselect-npm-5.1.1-667568f51c-1fdae11a39.zipis excluded by!**/.yarn/**,!**/*.zip.yarn/cache/use-sync-external-store-npm-1.4.0-176448bea1-08bf581a8a.zipis excluded by!**/.yarn/**,!**/*.zipyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (17)
.pnp.cjs(15 hunks)package.json(3 hunks)src/App.tsx(4 hunks)src/app/components/fulltime/FullTime.tsx(7 hunks)src/app/components/index.ts(1 hunks)src/app/components/lang/lang.en.json(1 hunks)src/app/components/lang/lang.ko.json(1 hunks)src/app/components/routemap/RouteMap.tsx(1 hunks)src/app/components/shuttle/Shuttle.tsx(4 hunks)src/app/data/index.ts(1 hunks)src/app/data/routemap/circleanimate.ts(1 hunks)src/app/data/shuttle/organizedTimetables.ts(1 hunks)src/app/network/shuttle.ts(1 hunks)src/app/reducer/store.ts(1 hunks)src/index.css(4 hunks)src/main.tsx(2 hunks)tailwind.config.js(3 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
src/app/components/fulltime/FullTime.tsx
[error] 299-299: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Cloudflare Pages
🔇 Additional comments (48)
src/app/components/index.ts (1)
2-2: Export statement looks goodThe export statement for the Shuttle component is properly configured.
src/app/network/shuttle.ts (1)
16-16:✅ Verification successful
API endpoint updated to v2
The API endpoint has been updated to use a versioned path (
/v2/timetable/), which follows good API versioning practices. Make sure that all the API consumers are aware of this change as it may break existing integrations.
🏁 Script executed:
#!/bin/bash # Search for any other places in the codebase that might be using the old API endpoint rg -e "/timetable/\$\{season\}" --type ts --type tsx -g "!src/app/network/shuttle.ts"Length of output: 114
#!/bin/bash # Re-run the search command with explicit file extension filters for ts and tsx files rg -g "*.ts" -g "*.tsx" "/timetable/\$\{season\}" --glob "!src/app/network/shuttle.ts"Based on the updated search command above, please verify manually that no other files reference the old endpoint. If the output remains empty, it confirms that the only reference has been updated to the new v2 endpoint.
API Endpoint v2 Update Confirmed
The API endpoint was updated to:
`/v2/timetable/${season}/${week}/${location}`All occurrences across TypeScript files (excluding the updated file) have been rechecked using explicit file filters, and no lingering references to the old endpoint were found. Please ensure that API consumers are informed about this update to avoid potential integration issues.
src/app/data/index.ts (1)
5-5: New type export for route map functionalityThe addition of the
CircleAnimatetype export aligns with the PR objective of adding route map functionality. This should help maintain type safety for components that use animated circles in the route map.src/app/components/lang/lang.en.json (1)
12-14: Localization entries added correctlyThese new entries for "Yesulin" and "Jungang" match with your existing localization structure and complement the existing entries on lines 25-26 and 41-42.
src/app/data/routemap/circleanimate.ts (1)
1-7: Type definition looks good for route animationThe
CircleAnimatetype is well-structured for handling animated route elements. The use ofMutableRefObjectfor direct DOM manipulation is appropriate for animations, though be mindful that such direct manipulation should be used carefully in React applications.src/app/components/lang/lang.ko.json (1)
12-14: Localization entries added correctlyThe Korean translations for "예술인" and "중앙역" are properly added and maintain consistency with the corresponding English translations in lang.en.json.
src/main.tsx (3)
8-8: Redux integration looks goodThe Redux Provider and store imports are correctly added to enable state management across the application.
Also applies to: 13-13
45-45: Z-index adjustment for snowfall componentChanging the z-index from 1 to 12 ensures the snowfall visual effect appears correctly layered relative to other components, likely including the new route map functionality.
51-53: Redux Provider correctly wraps the App componentThe Redux Provider implementation follows best practices by wrapping the App component, ensuring that the entire application has access to the Redux store.
package.json (1)
15-15: Addition of Redux and animation libraries looks good!The additions of Redux-related packages (
@reduxjs/toolkit,react-redux,redux) and animation libraries (react-transition-group) along with their type definitions are appropriate for implementing a route map with interactive state management and transitions.Also applies to: 30-30, 36-36, 38-38, 47-47
tailwind.config.js (3)
33-33: New arrow-color variable looks goodThis new color variable will help maintain consistent styling for arrows across different themes.
45-45: New chip-orange color is appropriateAdding this orange color expands the chip color palette, which will be useful for route indicators or status badges.
144-146: Height transition property is a good additionAdding the height transition property will enable smooth animations for expandable/collapsible elements, which are common in route map interfaces.
src/index.css (1)
31-31: Theme-specific arrow colors look goodThe arrow color variables are appropriately defined for each theme with colors that provide good contrast against their respective backgrounds.
Also applies to: 55-55, 79-79, 103-103
src/app/reducer/store.ts (3)
1-4: Redux setup looks goodThe imports are appropriate for setting up Redux with TypeScript support.
25-29: Store configuration looks goodThe store is properly configured with the actions reducer.
31-31: Action export looks goodCorrectly exporting the action creator for use in components.
src/app/components/fulltime/FullTime.tsx (5)
103-105: Good addition for handling 4 chip maximumThe additional condition for handling cases where
$maxChipsequals 4 is a necessary adjustment to properly accommodate the new directY chip type that has been added to the component.
180-183: Good addition of Yesulin route displayThe addition of a new
TimeBoxBodyGridto displayprops.directYaligns with the application's purpose to show the Yesulin bus route information.
185-185: Good conditional styling based on shuttle typeUsing conditional styling based on the
isShuttleIproperty provides clear visual distinction between different shuttle types.
262-262: Good addition of isShuttleI propertyAdding the
isShuttleIproperty to track internal shuttle routes enhances the component's ability to represent different shuttle types.
293-295: Good update to count logic for directYIncluding
directYin the count ensures that the component's layout correctly adapts to all available bus types.src/app/components/shuttle/Shuttle.tsx (4)
7-7: Good integration of Redux for state managementAdding the
useDispatchhook from react-redux enables the component to dispatch actions and interact with the Redux store.
26-26: Proper import of Redux action creatorsImporting the
updateActionsfunction from the Redux store setup is necessary for dispatching actions to update the store state.
340-342: Good update to ColoredChip styling logicAdding a condition for 'R' and 'NA' bus types to use orange background ensures visual consistency across the application.
349-349: Good initialization of Redux dispatchInitializing the
useDispatchhook at the component level makes it available throughout the component for state updates.src/App.tsx (6)
1-1: Good import organizationAdding useState to the import statement keeps related React hooks together, improving code organization.
5-5: Good use of Transition componentUsing the Transition component from react-transition-group is appropriate for creating smooth animated transitions between UI states.
14-14: Good component organization with RouteMapImporting the RouteMap component as a separate module promotes code reusability and separation of concerns.
121-124: Well-structured responsive transition stylingUsing twin.macro with transition properties for height creates a smooth animation effect, with responsive variants for different screen sizes.
135-138: Well-implemented animation for toggle buttonThe RouteToggleImage styled component provides smooth rotation animation for the toggle button, enhancing user experience.
199-199: Good state management for toggle behaviorUsing a boolean state variable to track the expanded/collapsed state of the route card is a clean approach.
.pnp.cjs (13)
27-38: Dependency Addition: @reduxjs/toolkit & @types/react-transition-group
The new entry for"@reduxjs/toolkit"(version2.5.0) is added as a virtual dependency, and the inclusion of"@types/react-transition-group"(version4.4.11) is now reflected in this block. Please verify that these versions and virtual references match those in your central dependency management (e.g. in package.json) and that they satisfy your project requirements.
59-70: Updated React-Redux & React-Transition-Group Dependencies
Here you’re introducing/updating dependencies for"react-redux"(version9.2.0) and"react-transition-group"(version4.4.5) with their respective virtual references. Please ensure that these specifications are consistent with the rest of your Redux setup and UI transition dependencies.
2732-2744: Addition of Babel Runtime Block
A new block is introduced for"@babel/runtime"at version7.26.0, along with its dependency on"regenerator-runtime". Confirm that this change integrates neatly with your Babel configuration and that the defined cache paths are correct.
4154-4168: New @types/react-transition-group Dependency Block
This block adds the type definitions for"@types/react-transition-group"using version4.4.11. Ensure that the pairing with"@types/react"(version18.3.1) is as intended and that this aligns with your TypeScript setup.
4212-4224: Inclusion of @types/use-sync-external-store
A dependency block for"@types/use-sync-external-store"(version0.0.6) has been added. Verify that this low-version number is deliberate and that it integrates well with your project's type system and dependency graph.
5835-5851: Addition of dom-helpers Dependency
The new block for"dom-helpers"(version5.2.1) brings along its own dependencies (including"@babel/runtime"and"csstype"). Please confirm that these dependencies are necessary for your UI components and that the version alignment with related libraries is correct.
7267-7276: Workspace Dependency Update
This section updates workspace-level dependency declarations that include"@reduxjs/toolkit"and"@types/react-transition-group". It’s important to ensure that these workspace references are consistent with your overall dependency configuration and do not introduce unintended duplications.
7299-7313: Consistent React-Related Dependency References
This block reiterates several react-related dependency entries (e.g."react-i18next","react-icons","react-redux","react-transition-group","redux"). While it appears deliberate, please verify that the duplication (or multi-location declaration) is managed correctly by your dependency resolver and does not lead to conflicts.
9437-9470: Refined React-Redux Configuration Block
The updated block for"react-redux"includes both a direct npm entry and a virtual entry with detailed peer dependency listings. Ensure that employing both entries (withlinkTypedifferences ofSOFTvs.HARD) is intentional and that they do not conflict during module resolution.
9626-9659: Dual Setup for React-Transition-Group
This segment provides two different configurations for"react-transition-group"—one from npm (usinglinkType: SOFT) and one virtual (usinglinkType: HARD). Double-check that this dual configuration is part of your intended module resolution strategy and does not cause version mismatches or redundant loading.
9733-9767: Redux & Redux-Thunk Updates
The dependency blocks for"redux"and"redux-thunk"have been updated here. Notice that"redux-thunk"is configured with both a direct npm entry (usinglinkType: SOFT) and a virtual alias (usinglinkType: HARD). Confirm that this setup is deliberate and harmonizes well with your Redux-based state management.
9874-9888: Addition of Reselect and Resolve Patch
A new configuration for"reselect"(version5.1.1) is introduced, which appears standard. Additionally, ensure that the patched"resolve"dependency is compatible with your current Plug'n'Play setup.
11334-11360: Updated use-sync-external-store Configuration
This segment updates the configuration for"use-sync-external-store", providing both a direct npm entry (withlinkType: SOFT) and a virtual entry (withlinkType: HARD). Please verify that this distinction is intentional and that it avoids any potential issues with duplicate module resolutions.src/app/components/routemap/RouteMap.tsx (3)
52-55: Verify transition logic for 'entered' and 'exit' states.The condition
(props.status === 'entered' || props.status === 'exit') ? tw\opacity-100` : tw`opacity-0`` may not reflect the full set of possible statuses. Please confirm the intended states and ensure consistent transitions.
204-223: Ensure robust offset reference and null checks inresponsiveLines.The code calculates positions using
refs.current[i]?.offsetLeftbut doesn't handle edge cases where offsetLeft could be undefined. Consider adding safeguard checks or fallback values to prevent potential runtime errors.
266-274: Confirm that the last removed child node is the correct one.The
circleAnimationRemovefunction always removes the last child, but there's no guarantee it's a ping circle if more children are added in the future. Adding checks to ensure you're removing the correct node can prevent accidental removal of other elements.
| @@ -278,12 +290,15 @@ const FullTime = () => { | |||
| if (single.jungang.length !== 0) { | |||
| single.count++ | |||
| } | |||
| if (single.directY.length !== 0) { | |||
| single.count++ | |||
| } | |||
|
|
|||
| filteredByType.push(single) | |||
| return <></> | |||
| // [{ time: '08', direct: ["08:00", "08:10", ...], circle: [], directY: ["08:20", "08:50"] }, { time: '09', direct: [], circle: [], directY: [] }, ...] | |||
There was a problem hiding this comment.
Missing key property for element in iterable
React requires a unique key for each element in an iterable to help with efficient rendering and updates.
- return <></>
+ return <React.Fragment key={hour}><></></React.Fragment>📝 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.
| // [{ time: '08', direct: ["08:00", "08:10", ...], circle: [], directY: ["08:20", "08:50"] }, { time: '09', direct: [], circle: [], directY: [] }, ...] | |
| return <React.Fragment key={hour}><></></React.Fragment> |
🧰 Tools
🪛 Biome (1.9.4)
[error] 299-299: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
| useEffect(() => { | ||
| if(timetable.data !== undefined){ | ||
| const filtered = timetable.data.filter((val) => isAfterCurrentTime(val)) | ||
| dispatch(updateActions(filtered[0])) | ||
| } | ||
|
|
||
| }, [currentTime, dispatch, timetable.data]) |
There was a problem hiding this comment.
Add error handling for empty filtered bus data
The current implementation does not handle the case where no buses meet the filtering criteria, which could lead to dispatching undefined values to Redux.
useEffect(() => {
if(timetable.data !== undefined){
const filtered = timetable.data.filter((val) => isAfterCurrentTime(val))
- dispatch(updateActions(filtered[0]))
+ if (filtered.length > 0) {
+ dispatch(updateActions(filtered[0]))
+ }
}
}, [currentTime, dispatch, timetable.data])📝 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.
| useEffect(() => { | |
| if(timetable.data !== undefined){ | |
| const filtered = timetable.data.filter((val) => isAfterCurrentTime(val)) | |
| dispatch(updateActions(filtered[0])) | |
| } | |
| }, [currentTime, dispatch, timetable.data]) | |
| useEffect(() => { | |
| if(timetable.data !== undefined){ | |
| const filtered = timetable.data.filter((val) => isAfterCurrentTime(val)) | |
| if (filtered.length > 0) { | |
| dispatch(updateActions(filtered[0])) | |
| } | |
| } | |
| }, [currentTime, dispatch, timetable.data]) |
| const circleAnimation = (props: CircleAnimate) => { | ||
| for(let i = props.index; i <= props.index+1; i++){ | ||
| if(props.ref.current[i].childNodes.length <= 1){ | ||
| if(i === 5 || (i === 4 && props.chipColor === 'bg-chip-blue')){ | ||
| const pingCircle = document.createElement('div') | ||
| pingCircle.classList.add('animate-ping', 'absolute', 'rounded-full', 'inline-flex', 'h-3', 'w-3', 'rt1:h-2.5', 'rt1:w-2.5', 'z-11', 'bg-chip-orange', 'mx-2'); | ||
| if(props.ref.current[i].title === 'skip'){ | ||
| props.ref.current[i+1]?.append(pingCircle) | ||
| } else{ | ||
| props.ref.current[i]?.append(pingCircle) | ||
| } | ||
| props.ref.current[i]?.append(pingCircle) | ||
| continue | ||
|
|
||
| } | ||
| const pingCircle = document.createElement('div') | ||
| pingCircle.classList.add('animate-ping', 'absolute', 'rounded-full', 'inline-flex', 'h-3', 'w-3', 'rt1:h-2.5', 'rt1:w-2.5', 'z-11', props.chipColor, 'mx-2'); | ||
| if(props.ref.current[i].title === 'skip'){ | ||
| props.ref.current[i+1]?.append(pingCircle) | ||
| } else{ | ||
| props.ref.current[i]?.append(pingCircle) | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Address potential double appending of pingCircle.
Inside the circleAnimation function, after appending the ping circle in the 'skip' or 'else' branch, line 252 unconditionally appends it again, causing duplicates. Remove or guard this second append to avoid unintended behavior.
Here's a suggested fix:
if(props.ref.current[i].title === 'skip'){
props.ref.current[i+1]?.append(pingCircle)
} else{
props.ref.current[i]?.append(pingCircle)
}
- props.ref.current[i]?.append(pingCircle)
continue📝 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 circleAnimation = (props: CircleAnimate) => { | |
| for(let i = props.index; i <= props.index+1; i++){ | |
| if(props.ref.current[i].childNodes.length <= 1){ | |
| if(i === 5 || (i === 4 && props.chipColor === 'bg-chip-blue')){ | |
| const pingCircle = document.createElement('div') | |
| pingCircle.classList.add('animate-ping', 'absolute', 'rounded-full', 'inline-flex', 'h-3', 'w-3', 'rt1:h-2.5', 'rt1:w-2.5', 'z-11', 'bg-chip-orange', 'mx-2'); | |
| if(props.ref.current[i].title === 'skip'){ | |
| props.ref.current[i+1]?.append(pingCircle) | |
| } else{ | |
| props.ref.current[i]?.append(pingCircle) | |
| } | |
| props.ref.current[i]?.append(pingCircle) | |
| continue | |
| } | |
| const pingCircle = document.createElement('div') | |
| pingCircle.classList.add('animate-ping', 'absolute', 'rounded-full', 'inline-flex', 'h-3', 'w-3', 'rt1:h-2.5', 'rt1:w-2.5', 'z-11', props.chipColor, 'mx-2'); | |
| if(props.ref.current[i].title === 'skip'){ | |
| props.ref.current[i+1]?.append(pingCircle) | |
| } else{ | |
| props.ref.current[i]?.append(pingCircle) | |
| } | |
| } | |
| } | |
| } | |
| const circleAnimation = (props: CircleAnimate) => { | |
| for(let i = props.index; i <= props.index+1; i++){ | |
| if(props.ref.current[i].childNodes.length <= 1){ | |
| if(i === 5 || (i === 4 && props.chipColor === 'bg-chip-blue')){ | |
| const pingCircle = document.createElement('div') | |
| pingCircle.classList.add('animate-ping', 'absolute', 'rounded-full', 'inline-flex', 'h-3', 'w-3', 'rt1:h-2.5', 'rt1:w-2.5', 'z-11', 'bg-chip-orange', 'mx-2'); | |
| if(props.ref.current[i].title === 'skip'){ | |
| props.ref.current[i+1]?.append(pingCircle) | |
| } else{ | |
| props.ref.current[i]?.append(pingCircle) | |
| } | |
| continue | |
| } | |
| const pingCircle = document.createElement('div') | |
| pingCircle.classList.add('animate-ping', 'absolute', 'rounded-full', 'inline-flex', 'h-3', 'w-3', 'rt1:h-2.5', 'rt1:w-2.5', 'z-11', props.chipColor, 'mx-2'); | |
| if(props.ref.current[i].title === 'skip'){ | |
| props.ref.current[i+1]?.append(pingCircle) | |
| } else{ | |
| props.ref.current[i]?.append(pingCircle) | |
| } | |
| } | |
| } | |
| } |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
src/app/components/shuttle/Shuttle.tsx (1)
423-429:⚠️ Potential issueAdd error handling for empty filtered bus data
The current implementation does not handle the case where no buses meet the filtering criteria, which could lead to dispatching undefined values to Redux.
useEffect(() => { if(timetable.data !== undefined){ const filtered = timetable.data.filter((val) => isAfterCurrentTime(val)) - dispatch(updateActions(filtered[0])) + if (filtered.length > 0) { + dispatch(updateActions(filtered[0])) + } } }, [currentTime, dispatch, timetable.data])src/app/components/fulltime/FullTime.tsx (1)
299-299:⚠️ Potential issueMissing key property for element in iterable
React requires a unique key for each element in an iterable to help with efficient rendering and updates.
- return <></> + return <React.Fragment key={hour}><></></React.Fragment>🧰 Tools
🪛 Biome (1.9.4)
[error] 299-299: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.(lint/correctness/useJsxKeyInIterable)
🧹 Nitpick comments (8)
src/app/components/lang/lang.en.json (1)
12-14: Consider standardizing translation key naming for locationsThe new translations for "yesul" and "jung" have been added, but they create an inconsistency with existing keys like "yesulin" and "jungang" that refer to the same locations.
For better maintainability, consider standardizing the key naming approach for locations:
- Either use the shorter form consistently ("yesul", "jung")
- Or use the full form consistently ("yesulin", "jungang")
src/app/reducer/store.ts (1)
5-8: Consider using PayloadAction type from Redux ToolkitInstead of defining a custom Action interface, consider using the built-in
PayloadAction<T>type from Redux Toolkit which is designed specifically for this purpose.-interface Action{ - type: string, - payload: SingleShuttleSchedule -} +import { PayloadAction } from "@reduxjs/toolkit"; // Then use PayloadAction<SingleShuttleSchedule> in your reducerpackage.json (1)
38-38: Dependency redundancy checkSince you're already using Redux Toolkit, which includes core Redux functionality, the explicit Redux dependency might be redundant. Consider if you really need both.
- "redux": "^5.0.1",src/app/components/fulltime/FullTime.tsx (2)
126-129: Commented out YesulinMinuteWrapperCommented code should be removed rather than left in the codebase. If this component is no longer needed, delete it entirely.
-// const YesulinMinuteWrapper = styled.span<{ $itemCount: number }>` -// ${tw`inline-block text-green-500`} -// ${({ $itemCount }) => ($itemCount === 0 ? tw`hidden` : undefined)} -// `
188-197: Added duplicate prevention for directY entriesThis logic ensures that times in both direct and directY lists aren't displayed twice. However, the implementation can be improved:
{props.direct.map((time, idx) => { - let isExist = false - props.directY.map((ytime) => { - time === ytime ? (isExist = true) : null - }) - return( - isExist ? null : + return props.directY.includes(time) ? null : ( <React.Fragment key={idx}> <span>{time} </span> </React.Fragment> ) })}This simplifies the code using the
includesmethod rather than iterating through the array with a map function that doesn't return anything..pnp.cjs (1)
3363-3396: Dual Resolution for @reduxjs/toolkit
This block introduces two entries for@reduxjs/toolkit—one direct (withlinkType: "SOFT") and one virtual incorporating additional dependencies (e.g.immer,@types/react, etc.). Please double‑check that having both resolutions is necessary and does not lead to conflicts in state management integration.src/app/components/routemap/RouteMap.tsx (2)
91-165: Consider extracting a reusable generator for route segments.
This block outlines repetitive logic to build multiple route segments (Direct, Cycle, Yesulin, Jungang). Refactoring the repetitive code into a single reusable function or component could reduce duplication and help maintain consistency.Here's an example sketch to unify the creation logic:
- // directInput, cycle, yesulin, jungang creation repeated + function createRouteStations( + count: number, + routeType: "direct" | "cycle" | "yesulin" | "jungang" + ): JSX.Element[] { + // generate relevant circles, handle 'EndCircle' logic, etc. + // return an array of <RouteStations/> elements + }
297-332: Consider usinguseLayoutEffectfor DOM measurements.
Line-drawing logic depends on element sizes and positions. Switching fromuseEffecttouseLayoutEffecthelps ensure measurements are taken before the browser repaints, preventing layout shift or flicker.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (13)
.yarn/cache/@babel-runtime-npm-7.26.0-9afa3c4ef6-9f4ea1c1d5.zipis excluded by!**/.yarn/**,!**/*.zip.yarn/cache/@reduxjs-toolkit-npm-2.5.0-9bdc99574d-b24ee7e89b.zipis excluded by!**/.yarn/**,!**/*.zip.yarn/cache/@types-react-transition-group-npm-4.4.11-dda183055d-a7f4de6e5f.zipis excluded by!**/.yarn/**,!**/*.zip.yarn/cache/@types-use-sync-external-store-npm-0.0.6-9e5c635381-a95ce33066.zipis excluded by!**/.yarn/**,!**/*.zip.yarn/cache/dom-helpers-npm-5.2.1-b38bb4470b-bed2341adf.zipis excluded by!**/.yarn/**,!**/*.zip.yarn/cache/immer-npm-10.1.1-973ae10d09-9dacf1e8c2.zipis excluded by!**/.yarn/**,!**/*.zip.yarn/cache/react-redux-npm-9.2.0-d87bb27c82-b3d2f89f46.zipis excluded by!**/.yarn/**,!**/*.zip.yarn/cache/react-transition-group-npm-4.4.5-98ea4ef96e-ca32d3fd21.zipis excluded by!**/.yarn/**,!**/*.zip.yarn/cache/redux-npm-5.0.1-f8e6b1cb23-a373f9ed65.zipis excluded by!**/.yarn/**,!**/*.zip.yarn/cache/redux-thunk-npm-3.1.0-6a8fdd3211-38c563db5f.zipis excluded by!**/.yarn/**,!**/*.zip.yarn/cache/reselect-npm-5.1.1-667568f51c-1fdae11a39.zipis excluded by!**/.yarn/**,!**/*.zip.yarn/cache/use-sync-external-store-npm-1.4.0-176448bea1-08bf581a8a.zipis excluded by!**/.yarn/**,!**/*.zipyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (17)
.pnp.cjs(15 hunks)package.json(3 hunks)src/App.tsx(4 hunks)src/app/components/fulltime/FullTime.tsx(7 hunks)src/app/components/index.ts(1 hunks)src/app/components/lang/lang.en.json(1 hunks)src/app/components/lang/lang.ko.json(1 hunks)src/app/components/routemap/RouteMap.tsx(1 hunks)src/app/components/shuttle/Shuttle.tsx(4 hunks)src/app/data/index.ts(1 hunks)src/app/data/routemap/circleanimate.ts(1 hunks)src/app/data/shuttle/organizedTimetables.ts(1 hunks)src/app/network/shuttle.ts(1 hunks)src/app/reducer/store.ts(1 hunks)src/index.css(4 hunks)src/main.tsx(2 hunks)tailwind.config.js(3 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
src/app/components/fulltime/FullTime.tsx
[error] 299-299: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Cloudflare Pages
🔇 Additional comments (54)
src/app/components/index.ts (1)
1-2: Looks good - index file export structure is correctThe export statement for the Shuttle component is properly formatted and follows the project's barrel file pattern.
src/app/network/shuttle.ts (1)
16-16: API endpoint updated to versioned pathThe API endpoint change from
/timetable/...to/v2/timetable/...indicates a proper versioning strategy.Please verify that the v2 endpoint is deployed and available in all environments where this code will run.
src/app/data/index.ts (1)
5-5: New CircleAnimate type export added for route map functionalityThe export of the CircleAnimate type maintains the established pattern for type exports and is directly related to the route map feature.
src/app/components/lang/lang.ko.json (2)
12-13: Translations added successfullyThe new "yesul" translation for "예술인" (artist) has been correctly added to the Korean language file.
14-14: Potential duplicate translationThe newly added "jung" translation for "중앙역" (central station) appears to duplicate the existing "cycle_ja" key (line 11) which already translates to "중앙역".
Please verify if these are indeed meant to represent the same location or if they have different contextual usages in the application.
src/app/data/shuttle/organizedTimetables.ts (1)
6-7:✅ Verification successful
New property added to the OrganizedTimetables type
The
isShuttleIboolean property appears to be used to mark specific shuttle routes. This change expands the data model correctly.
🏁 Script executed:
#!/bin/bash # Find where this boolean property is used throughout the codebase echo "Checking implementation of isShuttleI property..." rg "isShuttleI" --type=tsx --type=tsLength of output: 173
Action: Verify usage of the new
isShuttleIpropertyThe new property
isShuttleIhas been added to theOrganizedTimetablestype to mark specific shuttle routes as intended. Note that the original verification command failed due to an unrecognized file type ("tsx"). Please re-run the updated search command (as provided above) or perform a manual check to ensure that all usages ofisShuttleIare correctly handled across the codebase.
🏁 Script executed:
#!/bin/bash # Re-run search for the "isShuttleI" property in TS and TSX files using glob filters echo "Re-running search for isShuttleI using glob patterns..." rg "isShuttleI" -g "*.ts" -g "*.tsx"Length of output: 723
Action: Verified usage of
isShuttleIproperty across the codebase
- The new
isShuttleIboolean property is correctly added insrc/app/data/shuttle/organizedTimetables.ts(lines 6-7).- Its implementation is confirmed in
src/app/components/fulltime/FullTime.tsx, where the property correctly influences the UI (e.g., determining the chip's class and value assignments).The change appears intentional and the property is effectively used to mark specific shuttle routes. No further modifications are required.
src/app/data/routemap/circleanimate.ts (1)
1-7:❓ Verification inconclusive
New type for animation references looks good
The
CircleAnimatetype is well-structured for managing animated elements in the route map. Using React refs for directly manipulating DOM elements is appropriate for animations.
🏁 Script executed:
#!/bin/bash # Check for proper cleanup of refs to prevent memory leaks echo "Checking for ref cleanup in useEffect hooks when using CircleAnimate..." rg "CircleAnimate" -A 15 -B 5 --type=tsx | rg -A 3 -B 3 "useEffect"Length of output: 246
CircleAnimate Ref Cleanup Verification: Manual Check Needed
The new
CircleAnimatetype looks well-structured for managing animation references and correctly leverages React refs for DOM manipulation. However, our initial verification for proper cleanup inuseEffecthooks returned an error due to the unrecognized file type "tsx." To further assess this, please run the following updated script to search for cleanup logic in.tsxfiles:#!/bin/bash echo "Scanning for useEffect cleanup patterns in .tsx files referencing CircleAnimate..." rg -g "*.tsx" "CircleAnimate" | xargs -r rg -C 10 "useEffect"Review the output to ensure that any usage of
CircleAnimatein components is accompanied by proper cleanup (e.g., returning a cleanup function inuseEffecthooks) to prevent potential memory leaks.src/main.tsx (3)
8-8: Redux integration added properlyThe Redux Provider and store imports have been correctly added to enable global state management.
Also applies to: 13-13
51-53: Redux Provider properly wraps the applicationThe App component is correctly wrapped with the Redux Provider, making the store available to all components.
45-46:❓ Verification inconclusive
Increased z-index for snowfall effect
The snowfall z-index has been increased from a previous value to 12. This changes the stacking order and might affect how it appears in relation to other UI elements.
🏁 Script executed:
#!/bin/bash # Check for other z-index values in the codebase to ensure no conflicts echo "Checking for potential z-index conflicts..." rg "zIndex:" --type=tsx --type=ts | sort -nLength of output: 168
Action: Verify z-index stacking doesn’t conflict with other UI layers
The initial verification script error indicates that the TSX file type wasn’t recognized. Please update the search by adding a custom file type definition for TSX files. For example, try running:
#!/bin/bash echo "Searching zIndex values in TS and TSX files..." rg --type-add 'tsx:*.tsx' --type tsx "zIndex:" | sort -nOnce you confirm that there are no unexpected z-index conflicts, we can be confident that the increased value of 12 for the snowfall effect won’t adversely affect the UI layering.
src/app/reducer/store.ts (3)
1-4: Good choice of Redux ToolkitUsing Redux Toolkit with TypeScript provides a well-structured approach to state management. The import of the SingleShuttleSchedule type suggests proper typing integration with your data model.
25-29: Store configuration looks goodThe store is correctly configured with the actions reducer. This setup will work well with the Provider component to make the store available throughout the application.
31-31: Properly exported action creatorThe action creator is correctly exported using object destructuring from the generated actions object.
package.json (4)
15-15: Appropriate Redux Toolkit versionAdding Redux Toolkit with a recent version is a good choice for modern state management in React applications.
30-30: React-Redux integrationThe React-Redux package is necessary for connecting the Redux store to your React components. This version is compatible with the Redux Toolkit version you've added.
36-36: React Transition Group for animationsAdding react-transition-group will enable smooth UI transitions, which works well with the height transition property added to Tailwind config.
47-47: Type definitions for transitionsGood practice adding the type definitions for react-transition-group to ensure type safety when using the transition components.
src/index.css (1)
31-31: Consistent theme color variablesAdding the arrow color variables to all themes maintains consistency in the theming system. The color choices appear to provide good contrast within each theme's palette.
Also applies to: 55-55, 79-79, 103-103
tailwind.config.js (3)
33-33: CSS variable reference in TailwindCorrectly mapping the Tailwind color to the CSS variable allows for theme-based styling that will automatically update when the theme changes.
45-45: New color for chip componentsThe addition of chip-orange suggests you're expanding the UI component set with new visual indicators. The color choice (#FFAB40) provides good visibility.
144-146: Height transition propertyAdding a specific transition property for height will enable smooth animations when elements change height. This is likely used with the react-transition-group package added to dependencies.
src/app/components/shuttle/Shuttle.tsx (4)
7-7: New Redux dependency importThe addition of
useDispatchfrom React Redux indicates a shift to Redux for state management. This aligns with the global application changes mentioned in the summary.
26-26: Added Redux action importThe import of
updateActionsfrom the store file supports the new Redux state management functionality.
340-342: Added styling for bus types R and NAGood enhancement to provide visual distinction for the 'R' (Return) and 'NA' bus types with orange background styling.
349-349: Initialized Redux dispatchThe
useDispatchhook is properly initialized to enable Redux actions.src/app/components/fulltime/FullTime.tsx (6)
103-105: Added support for 4 chip typesGood addition to handle cases where there are 4 different chip types with appropriate height adjustments.
180-183: Added support for directY route typeGood addition of a dedicated grid for Yesulin direct routes, improving organization of the different route types.
185-185: Enhanced Chip component with conditional stylingThe Chip component now dynamically changes styling based on the shuttle type, improving visual distinction between different routes.
262-263: Added isShuttleI flag for residence routesThe code correctly identifies and flags residence routes (type 'R') with the
isShuttleIproperty, which is then used for styling.Also applies to: 279-281
293-295: Added count logic for directY entriesThis properly increments the count for Yesulin direct routes, ensuring correct height calculations for the TimeBox component.
317-318: Enhanced conditional rendering logicThe rendering condition now properly checks all route types (direct, circle, directY, and jungang) before deciding whether to render the TimeBox.
src/App.tsx (6)
1-1: Added React Transition Group for animationsGood addition of the Transition component for smooth animations when expanding/collapsing the route map.
Also applies to: 5-5
14-14: Added RouteMap component importThis import supports the new route map feature that shows bus routes visually.
121-124: Enhanced RouteIndexCardView with transition stateThe RouteIndexCardView component now properly accepts a status prop to control its height during transitions. The twin.macro styling is well-implemented for responsive behavior.
130-138: Added containers and controls for route mapThe RouteIndexContainer and RouteToggleImage components are well-designed to handle the transition states with proper opacity and rotation animations.
199-199: Added state for route card expansionThe routeCardClick state variable properly manages the expanded/collapsed state of the route map.
448-478: Implemented transition system for route mapThe implementation of the Transition component from react-transition-group creates a smooth animation when toggling the route map. The code correctly:
- Uses the routeCardClick state to control the transition
- Passes the transition state to child components
- Handles the toggle with an onClick handler
- Renders the RouteMap component with the appropriate state
This is a well-implemented feature that enhances the user experience.
.pnp.cjs (14)
27-38: Dependency Addition – Redux Toolkit & React Transition Group Types
The new entry for@reduxjs/toolkit(v2.5.0) and the addition of@types/react-transition-group(v4.4.11) support enhanced Redux integration and UI transition type safety. Please ensure these versions are compatible with your overall dependency set.
59-73: React Ecosystem Dependencies Update
This hunk updates several React-related dependencies (includingreact-reduxat v9.2.0 andreact-transition-groupat v4.4.5). Verify that these virtual package designations satisfy your project’s bundling and runtime expectations.
2732-2745: Babel Runtime & Regenerator Block Restructuring
The updated structure for the@babel/runtimedependency (with version 7.26.0) now groups it alongsideregenerator-runtime. Confirm that this configuration is intentional and aligns with Yarn PnP guidelines.
4154-4169: Type Definitions for React Transition Group & Resolve
Adding@types/react-transition-groupensures proper type checking for transition components, while the inclusion of@types/resolve(v1.20.2) should be reviewed to confirm its need.
4212-4226: Addition of @types/use-sync-external-store and ESLint Plugin Block
The new@types/use-sync-external-store(v0.0.6) enhances support for React’s external store hook, and the ESLint plugin entry continues to enforce code quality. Please verify that these versions are well‑aligned with your current React and ESLint configurations.
5835-5851: Dom-Helpers Dependency Block Added
The block fordom-helpers(v5.2.1) indicates reliance on utility functions for DOM manipulation—likely supporting UI transitions—and it appears alongside adot-casedependency. Ensure that these libraries are both actively used and do not overlap in functionality.
7267-7278: Workspace Dependency Consolidation
This hunk lists the workspace’s overall dependencies. Notably, it reiterates the@reduxjs/toolkitentry (v2.5.0) along with several type definitions. Confirm that these repeated entries are synchronized with the dependency management strategy across your project.
7299-7313: React Dependencies Consistency Check
A further declaration of React ecosystem dependencies is observed, including an updatedreact-reduxentry. Please verify that these sections are consistent and intentional across different dependency scopes in your PnP configuration.
7396-7410: Introducing Immer – Evaluate Overlap with Immutable
A new block forimmer(v10.1.1) is now present immediately before theimmutableblock. As noted in previous reviews, consider whether both libraries are necessary given their overlapping approaches to state immutability.
9437-9470: React-Redux Resolution Refinement
This section reconfigures thereact-reduxdependency with both npm and virtual resolutions, using differing link types (SOFTandHARD). Please confirm that this dual resolution is required for proper module resolution and does not cause unexpected behavior in your state management setup.
9626-9661: Dual Block for React Transition Group
The configuration forreact-transition-groupnow includes both a SOFT-linked and a virtual entry, with an extensive dependency list in the latter. Verify that the additional dependency tree provided by the virtual entry is necessary for the enhanced functionality you require.
9733-9769: Redux and Redux-Thunk Configuration Check
This hunk establishes theredux(v5.0.1) andredux-thunk(v3.1.0) dependencies, with dual (npm and virtual) setups for thunk. Ensure both entries are coupled correctly with their peer dependency requirements and that the mixed link types are intentional.
9874-9888: New Blocks for Reselect and Patched Resolve
A new dependency forreselect(v5.1.1) is added, and a patched entry forresolveis introduced. Please review whether the patch forresolveis still needed for your environment and that these new additions align with your performance and bundle size goals.
11334-11361: Use-Sync-External-Store & Util-Deprecate Additions
This final hunk updates theuse-sync-external-storedependency (v1.4.0) with both soft and virtual resolutions, and introducesutil-deprecate(v1.0.2). Verify that the dual resolution for the external store hook is necessary for compatibility with React and thatutil-deprecateposes no security concerns.src/app/components/routemap/RouteMap.tsx (3)
242-263: Remove duplicated pingCircle append logic.
This code is appending the same pingCircle again on line 252 after it was already appended in the 'skip' or 'else' branch. Removing or guarding this second append prevents duplicate circles.Apply the previously suggested fix:
if(props.ref.current[i].title === 'skip'){ props.ref.current[i+1]?.append(pingCircle) } else{ props.ref.current[i]?.append(pingCircle) } - props.ref.current[i]?.append(pingCircle) continue
286-296: Check effect dependencies for directInput, directLineInput, and updateLines.
These hooks handle layout updates, translations, and event listeners. Confirm that the dependency arrays inuseEffectsufficiently capture all relevant references (particularly forupdateLines). If any upstream changes to these functions or external values are not triggering the effect, consider adding them to dependencies to maintain consistency in rendering.
204-223:❓ Verification inconclusive
Verify array boundaries in responsiveLines.
Wheniequalslines.current.length, the code readsrefs.current[i], which may be undefined and lead to out-of-bounds indexing. Ensure that the number of lines always matches the number of route stations, or add bounds-checks inside the loop to avoid unintended behavior in edge cases.Below is a possible script to confirm these arrays have matching lengths:
🏁 Script executed:
#!/bin/bash # Search for references to 'linedir'/'refdir' and check if they always store the same number of items rg "linedir\.current\[(\w+)\]" -A 3 rg "refdir\.current\[(\w+)\]" -A 3Length of output: 1465
Ensure consistent array sizing for adjacent element mapping.
It appears that the route station (refs) and line (lines) arrays are populated in separate loops—with station refs being created over 6 iterations and line refs over 5 iterations. Since theresponsiveLinesfunction iterates from 1 up tolines.current.length(i.e. 5) and accesses bothrefs.current[i-1]andrefs.current[i], it’s assumed thatrefs.currenthas exactly one more element thanlines.current. Please verify that this invariant (i.e.refs.current.length === lines.current.length + 1) is maintained across all use cases. If there’s any chance of a mismatch, consider adding bounds-checks or adjusting the loop conditions to guard against accessing undefined elements.
| const actions = createSlice({ | ||
| name: 'actions', | ||
| initialState: [] as SingleShuttleSchedule[], | ||
| reducers: { | ||
| updateActions: (state, action: Action) => { | ||
| while(state.length > 0){ | ||
| state.pop() | ||
| } | ||
| if(action.payload !== undefined){ | ||
| state.push(action.payload) | ||
| } | ||
| } | ||
| } | ||
| }) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Optimize state clearing logic
The current approach to clearing the state array using a while loop is inefficient. Redux Toolkit's createSlice uses Immer under the hood, which allows for simpler state mutations.
updateActions: (state, action: Action) => {
- while(state.length > 0){
- state.pop()
- }
+ state.length = 0; // This is more efficient for clearing an array
if(action.payload !== undefined){
state.push(action.payload)
}
}📝 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 actions = createSlice({ | |
| name: 'actions', | |
| initialState: [] as SingleShuttleSchedule[], | |
| reducers: { | |
| updateActions: (state, action: Action) => { | |
| while(state.length > 0){ | |
| state.pop() | |
| } | |
| if(action.payload !== undefined){ | |
| state.push(action.payload) | |
| } | |
| } | |
| } | |
| }) | |
| const actions = createSlice({ | |
| name: 'actions', | |
| initialState: [] as SingleShuttleSchedule[], | |
| reducers: { | |
| updateActions: (state, action: Action) => { | |
| state.length = 0; // This is more efficient for clearing an array | |
| if(action.payload !== undefined){ | |
| state.push(action.payload) | |
| } | |
| } | |
| } | |
| }) |
There was a problem hiding this comment.
PR Overview
This PR introduces an interactive shuttle route map with dynamic animations, smoother transitions, and responsive visuals while integrating real-time shuttle schedule updates via Redux. Key changes include:
- Implementation of a new RouteMap component with animation and responsive layout.
- Integration of Redux store logic for shuttle schedule actions.
- Updates to network endpoints, UI styling, and minor enhancements in shuttle and fulltime components.
Reviewed Changes
| File | Description |
|---|---|
| src/app/reducer/store.ts | Added a Redux slice and store configuration for shuttle schedule data. |
| src/app/data/routemap/circleanimate.ts | Defined a type for circle animation props used in the route map. |
| src/app/components/routemap/RouteMap.tsx | Created a new interactive route map with responsive layout and animation. |
| src/App.tsx | Integrated the RouteMap component with Transition and updated layout. |
| src/main.tsx | Wrapped the app in a Redux Provider and adjusted z-index for snowfall. |
| src/app/components/fulltime/FullTime.tsx | Updated styling and added a new flag for isShuttleI in time box views. |
| src/app/components/shuttle/Shuttle.tsx | Added dispatch to update shuttle actions, and updated chip color logic. |
| src/app/network/shuttle.ts | Changed API endpoint to a versioned URL. |
| src/app/data/shuttle/organizedTimetables.ts | Updated organized timetables type to include a shuttle indicator flag. |
| src/app/data/index.ts | Exported the CircleAnimate type for wider usage. |
Copilot reviewed 30 out of 30 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
src/app/components/shuttle/Shuttle.tsx:426
- [nitpick] Dispatching updateActions(filtered[0]) without a check may result in an undefined payload if the filtered array is empty. Consider adding a guard to ensure that filtered[0] exists before dispatching.
dispatch(updateActions(filtered[0]))
| } else{ | ||
| props.ref.current[i]?.append(pingCircle) | ||
| } | ||
| props.ref.current[i]?.append(pingCircle) |
There was a problem hiding this comment.
The pingCircle element is appended twice when i === 5 or (i === 4 && props.chipColor === 'bg-chip-blue'), which may result in duplicate DOM nodes. Please remove the redundant append to ensure the element is added only once.
| props.ref.current[i]?.append(pingCircle) |
Copilot is powered by AI, so mistakes are possible. Review output carefully before use.
| import { useTranslation } from 'react-i18next' | ||
| import { BrowserRouter, Link, Navigate, Route, Routes } from 'react-router-dom' | ||
| import PullToRefresh from 'react-simple-pull-to-refresh' | ||
| import {Transition} from 'react-transition-group' |
There was a problem hiding this comment.
Please format the file (L1, L5)
src/app/components/index.ts
Outdated
There was a problem hiding this comment.
This file change is not necessary
There was a problem hiding this comment.
We manage our state with useState. Please explain why we need an another state management library.
There was a problem hiding this comment.
The file name should be in lowerCamelCase pattern.
| @@ -40,13 +42,15 @@ ReactDOM.createRoot(root).render( | |||
| wind={[-0.5, 0.5]} | |||
| radius={[14.0, 16.0]} | |||
| style={{ | |||
| zIndex: 1, | |||
| zIndex: 12, | |||
There was a problem hiding this comment.
Please format this file as well.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
src/app/reducer/store.ts (1)
1-31: 🛠️ Refactor suggestionSetting up Redux store for shuttle schedules
You've created a basic Redux store with actions to update shuttle schedules. The implementation is properly structured, but there are opportunities for improvement.
Optimize the state clearing logic:
updateActions: (state, action: Action) => { - while(state.length > 0){ - state.pop() - } + state.length = 0; // This is more efficient for clearing an array if(action.payload !== undefined){ state.push(action.payload) } }Additionally:
- Consider adding comments explaining the purpose of this store
- The Action interface should extend PayloadAction from Redux Toolkit
- Add typed selectors for accessing the state
src/app/components/shuttle/Shuttle.tsx (1)
423-429:⚠️ Potential issueSafeguard against empty array before dispatching.
Iffilteredis empty, you'll dispatchupdateActions(undefined). Adding a quick check ensures no undefined payload is dispatched.useEffect(() => { if(timetable.data !== undefined){ const filtered = timetable.data.filter((val) => isAfterCurrentTime(val)) - dispatch(updateActions(filtered[0])) + if (filtered.length > 0) { + dispatch(updateActions(filtered[0])) + } } }, [currentTime, dispatch, timetable.data])
🧹 Nitpick comments (7)
src/app/network/shuttle.ts (1)
16-16: API endpoint updated to v2The API endpoint has been updated to use a versioned path (/v2/). Consider adding a comment explaining the reason for this version change and any differences in the returned data structure (such as the new
isShuttleIproperty mentioned in the summary).- `/v2/timetable/${season}/${week}/${location}` + `/v2/timetable/${season}/${week}/${location}` // Updated to v2 API which includes isShuttleI propertysrc/app/components/lang/lang.en.json (1)
12-14: New translation keys for route mapThe addition of translation keys for "Yesulin" and "Jungang" locations will support the new route map feature. Consider removing the extra blank line at line 14 to maintain consistent formatting with the rest of the file.
"yesul": "Yesulin", "jung": "Jungang", -tailwind.config.js (1)
144-146: Added height transition propertyThis addition enables smooth animated transitions when element heights change, improving the user experience for expanding/collapsing elements. Consider documenting where this transition will be used to help other developers understand its purpose.
src/app/reducer/store.ts (1)
25-29: Define store typingMissing type definition for the Redux store. Add RootState and AppDispatch types to improve TypeScript integration across the application.
export const store = configureStore({ reducer: { actions: actions.reducer } }) +// Define types for state and dispatch +export type RootState = ReturnType<typeof store.getState> +export type AppDispatch = typeof store.dispatchsrc/App.tsx (1)
448-479: Well-structured Transition usage.
Consider adding transitions for theexitingorexitedstates if you need more refined behavior. Otherwise, the conditional rendering looks fine.src/app/components/fulltime/FullTime.tsx (2)
126-129: Commented-out component.
If it's unused, consider removing it rather than commenting it out to keep the codebase clean.-// const YesulinMinuteWrapper = styled.span<{ $itemCount: number }>` -// ${tw`inline-block text-green-500`} -// ${({ $itemCount }) => ($itemCount === 0 ? tw`hidden` : undefined)} -// `
189-197: Consider a more direct approach with.some(...).
Instead of mutatingisExistwithinmap, use array methods like.some(...)for clarity.const isExist = props.directY.some((ytime) => ytime === time) return ( isExist ? null : <React.Fragment key={idx}> <span>{time} </span> </React.Fragment> )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (13)
.yarn/cache/@babel-runtime-npm-7.26.0-9afa3c4ef6-9f4ea1c1d5.zipis excluded by!**/.yarn/**,!**/*.zip.yarn/cache/@reduxjs-toolkit-npm-2.5.0-9bdc99574d-b24ee7e89b.zipis excluded by!**/.yarn/**,!**/*.zip.yarn/cache/@types-react-transition-group-npm-4.4.11-dda183055d-a7f4de6e5f.zipis excluded by!**/.yarn/**,!**/*.zip.yarn/cache/@types-use-sync-external-store-npm-0.0.6-9e5c635381-a95ce33066.zipis excluded by!**/.yarn/**,!**/*.zip.yarn/cache/dom-helpers-npm-5.2.1-b38bb4470b-bed2341adf.zipis excluded by!**/.yarn/**,!**/*.zip.yarn/cache/immer-npm-10.1.1-973ae10d09-9dacf1e8c2.zipis excluded by!**/.yarn/**,!**/*.zip.yarn/cache/react-redux-npm-9.2.0-d87bb27c82-b3d2f89f46.zipis excluded by!**/.yarn/**,!**/*.zip.yarn/cache/react-transition-group-npm-4.4.5-98ea4ef96e-ca32d3fd21.zipis excluded by!**/.yarn/**,!**/*.zip.yarn/cache/redux-npm-5.0.1-f8e6b1cb23-a373f9ed65.zipis excluded by!**/.yarn/**,!**/*.zip.yarn/cache/redux-thunk-npm-3.1.0-6a8fdd3211-38c563db5f.zipis excluded by!**/.yarn/**,!**/*.zip.yarn/cache/reselect-npm-5.1.1-667568f51c-1fdae11a39.zipis excluded by!**/.yarn/**,!**/*.zip.yarn/cache/use-sync-external-store-npm-1.4.0-176448bea1-08bf581a8a.zipis excluded by!**/.yarn/**,!**/*.zipyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (17)
.pnp.cjs(15 hunks)package.json(3 hunks)src/App.tsx(4 hunks)src/app/components/fulltime/FullTime.tsx(7 hunks)src/app/components/index.ts(1 hunks)src/app/components/lang/lang.en.json(1 hunks)src/app/components/lang/lang.ko.json(1 hunks)src/app/components/routemap/RouteMap.tsx(1 hunks)src/app/components/shuttle/Shuttle.tsx(4 hunks)src/app/data/index.ts(1 hunks)src/app/data/routemap/circleanimate.ts(1 hunks)src/app/data/shuttle/organizedTimetables.ts(1 hunks)src/app/network/shuttle.ts(1 hunks)src/app/reducer/store.ts(1 hunks)src/index.css(4 hunks)src/main.tsx(2 hunks)tailwind.config.js(3 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
src/app/components/fulltime/FullTime.tsx
[error] 299-299: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Cloudflare Pages
🔇 Additional comments (42)
src/app/data/index.ts (1)
5-5: LGTM: New type export for route map featureThis addition properly exports the CircleAnimate type that will be used for the new route map feature. The export follows the existing pattern and maintains alphabetical ordering.
src/app/components/index.ts (1)
2-2: This file change appears unnecessaryThe export statement has been removed and then re-added with no functional changes.
src/app/data/shuttle/organizedTimetables.ts (1)
6-7: Type extension looks good.The addition of the
isShuttleIboolean property to theOrganizedTimetablestype is a clean extension that will help identify a specific shuttle type. This property will likely be used for conditional rendering or filtering in the UI components.src/app/data/routemap/circleanimate.ts (1)
1-7: The file name should follow lowerCamelCase pattern.The type definition itself looks good with appropriate React typing for the ref and clear property naming. However, the file name should follow lowerCamelCase pattern to maintain consistency with TypeScript conventions.
Consider renaming the file to
circleAnimate.tsto follow the project's naming conventions.src/app/components/lang/lang.ko.json (1)
12-14: Translation additions look good.The new Korean translations for "yesul" (예술인) and "jung" (중앙역) match with their English counterparts "Yesulin" and "Jungang" mentioned in the summary. These additions ensure consistency across language support for the route map feature.
src/main.tsx (3)
8-8: Redux integration looks good.Adding the Redux Provider and store imports is the correct approach for integrating Redux state management into the application.
Also applies to: 13-13
45-45: Why is this z-index changed?The z-index of the Snowfall component has been increased from 1 to 12. This significant change in stacking order might affect the visibility of other UI elements.
Could you explain the reasoning behind this z-index change? Is it related to ensuring the snowfall appears above newly added route map components?
51-53: Redux Provider implementation looks good.The App component is correctly wrapped with the Redux Provider, making the store accessible throughout the component tree.
tailwind.config.js (2)
33-33: Consistent theme variable for arrow color addedThe addition of the 'arrow-color' variable utilizing CSS custom properties enhances theme consistency across the application. This aligns well with the corresponding CSS variables added in the index.css file.
45-45: New color added to chip paletteThe 'chip-orange' color expands the available chip color options, which now include red, blue, green, purple, and orange. This provides more visual differentiation options for UI elements.
package.json (2)
15-15: Introducing Redux for state managementYou're adding Redux and its toolkit for state management, but there's already a question from a team member about why another state management solution is needed when useState is already being used. Please provide clear documentation on why this architectural change is necessary and what benefits it brings to the project.
Consider searching for documentation on the differences and benefits:
When to use Redux vs React useState for state management?Also applies to: 30-30, 38-38
36-36: Adding transition group capabilitiesThe addition of react-transition-group and its TypeScript definitions enhances the application's animation capabilities. This library will allow for more sophisticated component transitions than CSS alone.
Also applies to: 47-47
src/index.css (1)
31-31: Added theme-specific arrow colorsThe new arrow color variables are consistently applied across all themes, maintaining the color scheme of each theme. This enables themed arrows that appropriately match the visual language of each theme variant.
Also applies to: 55-55, 79-79, 103-103
src/app/components/shuttle/Shuttle.tsx (3)
7-7: No concerns regarding the use ofuseDispatch.
The import fromreact-reduxaligns correctly with the subsequent usage in the component.
26-26: Action import is consistent with usage.
EnsureupdateActionsis correctly handling the expected payload in the Redux store.
349-349: Proper instantiation of Redux dispatch.
No issues with setting upconst dispatch = useDispatch().src/App.tsx (7)
1-1: Lazy import and Suspense usage look good.
No issues with the new import statement.
5-5: React Transition Group import is valid.
No further action needed.
10-10: SVG import consistent with new design.
Implementation appears fine.
14-14: New RouteMap import.
Ensure that the component is thoroughly tested for all transitions.
121-123: Conditional height transition forRouteIndexCardView.
Implementation of transition states looks good. Remember to verify intermediate states likeenteringorexiting.
135-138: Arrow rotation logic seems correct.
The toggling ofrotate-0vsrotate-180is well-implemented for transitions.
199-199: Introductory state for route card click.
No immediate issues spotted with this new state variable.src/app/components/fulltime/FullTime.tsx (2)
104-105: New height for 4-chip layouts.
Supports an additional row neatly. Implementation is fine.
180-183: Direct minutes for 'yesul' route recognized.
The usage ofprops.directYis proper. No issues here.src/app/components/routemap/RouteMap.tsx (2)
242-253: Double append ofpingCircle.
This was previously flagged; the ping circle is appended again after the conditional block, causing duplicates.if (props.ref.current[i].title === 'skip') { props.ref.current[i+1]?.append(pingCircle) } else { props.ref.current[i]?.append(pingCircle) } -props.ref.current[i]?.append(pingCircle)
1-383: Overall newRouteMapcomponent.
Good usage of references and dynamic line creation.
Keep an eye on transitions forstatus=exiting, if that state is needed.
Thoroughly test for differenttabvalues ensuring circles render accurately..pnp.cjs (15)
27-38: Dependency Additions: @reduxjs/toolkit & @types/react-transition-group
The new entry for@reduxjs/toolkit(using a virtual reference with version2.5.0) and the inclusion of@types/react-transition-group(version4.4.11) are added to the package dependencies. Please verify that the use of a virtual reference for these packages is intentional and that these versions align with your project’s requirements and the corresponding entries in your package manifest.
59-73: Review of React-Related Dependencies
In this hunk, the dependency entries for packages likereact-redux,react-transition-group, andreduxare updated. Notably,react-reduxnow uses a virtual reference with version9.2.0, andreduxis referenced directly via npm. Please double-check that the mix of virtual versus direct npm sources (as well as the chosen link types) is intentional and consistent with your resolution strategy across the project.
2732-2745: Grouping of @babel/runtime and regenerator-runtime
The updated block now groups@babel/runtime(version7.26.0) with its dependencyregenerator-runtime(version0.14.1), using a HARD link type. Verify that this grouping meets your build and runtime expectations and that the dependency versions are correct and consistent with your overall configuration.
3363-3396: Dual Resolution for @reduxjs/toolkit
This hunk introduces a new configuration block for@reduxjs/toolkitwith two entries:
- One using a direct npm reference (with linkType
SOFT), and- Another using a virtual reference (with linkType
HARD) that also lists several peer dependencies (e.g.@types/react,immer,react-redux, etc.).Please verify that both resolution strategies are required and that the divergent link types are intentional. Ensure the peer dependencies are accurate and that they align with your usage patterns elsewhere in the repository.
4154-4167: Addition of @types/react-transition-group Block
A new dedicated block for@types/react-transition-group(version4.4.11) has been added, explicitly listing its dependency on@types/react. Confirm that this separate block does not conflict with any earlier mention of this package in the dependencies and that the HARD link type is the intended configuration for type definitions.
4212-4224: New Entry for @types/use-sync-external-store
The dependency for@types/use-sync-external-store(version0.0.6) is introduced here with a HARD link type. This addition should ensure proper type support for external store usage. Verify that version0.0.6is compatible with your codebase and that you don’t have any conflicting type definitions elsewhere.
5835-5851: Introduction of dom-helpers Dependency
The new package block fordom-helpers(version5.2.1) is added with its required dependencies (@babel/runtimeandcsstype). Please ensure that this dependency and its associated versions are compatible with your UI components. Also, check that the following dependency block fordot-case(starting at line 5849) remains consistent with your project’s needs.
7267-7278: Workspace Dependency Block Update
Within the workspace dependency list, updates for packages such as@reduxjs/toolkitand@types/react-transition-groupappear. Confirm that these entries (especially the updated virtual reference for@reduxjs/toolkitand the type definitions) are consistent with the previously defined versions and resolve to the intended modules.
7299-7313: Consistency Check for React-Related Dependencies
This hunk repeats entries for several React-related packages:
react-reduxnow appears with an updated virtual reference.react-transition-groupandreduxare also updated.Please verify that these repeated dependency declarations are necessary and that their versions and link types match the rest of your dependency graph. Consolidation might be worth considering if they are redundant.
7396-7408: Introduction of Immer Dependency Block
A new block forimmer(version10.1.1) is added next to the already existingimmutabledependency. As noted in prior reviews, having bothimmerandimmutablecould be redundant since they offer overlapping paradigms for immutable state management. Please re‑evaluate if both libraries are necessary for your project.
9437-9470: React-Redux Dual Resolution
This hunk defines another block forreact-redux, now providing two resolution paths:
- A direct npm version (linkType
SOFT), and- A virtual version with a comprehensive dependency and peer list (linkType
HARD).Double-check that having both versions does not cause resolution conflicts and that the peer dependency lists are correctly maintained.
9626-9659: Dual Configuration for react-transition-group
The dependency block forreact-transition-groupnow includes:
- An npm entry with a SOFT link type, and
- A virtual entry with a HARD link type and an extended list of dependencies and peers.
Confirm that the rationale for maintaining dual entries is solid and that it won’t result in unexpected module resolution behavior.
9733-9769: Review of redux and redux-thunk Blocks
New dependency blocks forreduxandredux-thunkare introduced. Thereduxdependency consistently uses a HARD link type, whileredux-thunkis provided via both a direct npm reference and a virtual reference (with a HARD link type and associated peers). Please verify that these configurations are consistent with your project’s state management strategy and that there are no conflicts between the dual resolutions.
9874-9888: Addition of reselect and Patched Resolve
A fresh block forreselect(version5.1.1) has been added, supporting Redux selector optimizations. Additionally, a patched version ofresolveis referenced—please review that the patch (noted by the unique patch specifier) is still applicable and necessary.
11334-11361: Configuration for use-sync-external-store & util-deprecate
This final hunk introduces a detailed dependency configuration foruse-sync-external-storewith both npm (using a SOFT link type) and virtual entries (using a HARD link type) along with appropriate peer dependencies for React. It also adds a block forutil-deprecate. Please verify that the dual configuration foruse-sync-external-storeis intentional and that the overall dependency versions and link types are consistent with your resolution strategy.
| const RouteIndexContainer = styled.div<{status: string}>` | ||
| ${tw`absolute top-0 inset-0 flex place-content-center items-center transition ease-in-out duration-300`} | ||
| ${(props) => props.status === 'exited' ? tw`opacity-100` : tw`opacity-0`} | ||
| ${(props) => props.status === 'entered' ? tw`hidden`: tw``} | ||
| ` |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Verify logic in extended states.
Hiding the container on entered and making it fully opaque on exited may invert typical logic. Confirm this aligns with your desired behavior.
|
Check out your Lighthouse Report: https://lighthouse.hybus.app/app/projects/bushanyang-production/dashboard |
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/app/components/shuttle/Shuttle.tsx (1)
423-429:⚠️ Potential issueAdd error handling for empty filtered bus data
The current implementation does not handle the case where no buses meet the filtering criteria, which could lead to dispatching undefined values to Redux.
useEffect(() => { if(timetable.data !== undefined){ const filtered = timetable.data.filter((val) => isAfterCurrentTime(val)) - dispatch(updateActions(filtered[0])) + if (filtered.length > 0) { + dispatch(updateActions(filtered[0])) + } } }, [currentTime, dispatch, timetable.data])
🧹 Nitpick comments (15)
src/app/network/shuttle.ts (1)
16-16: Document the API version upgrade.The endpoint has been changed to use
/v2/timetable/instead of the previous version. Consider adding a comment to explain the reason for this version upgrade and any differences in the response format that consumers of this API need to be aware of.- `/v2/timetable/${season}/${week}/${location}` + // Using v2 API which provides support for route map visualization + `/v2/timetable/${season}/${week}/${location}`src/app/data/shuttle/organizedTimetables.ts (1)
6-7: Document the purpose of new properties.The addition of
jungangandisShuttleIproperties expands the data structure, but their purpose isn't immediately clear from the type definition. Consider adding TypeScript documentation comments to explain what these properties represent and how they should be used.export type OrganizedTimetables = { time: string direct: Array<string> circle: Array<string> directY: Array<string> + /** Array of times for Jungang line shuttle stops */ jungang: Array<string>, + /** Indicates whether this timetable entry is for Shuttle I route */ isShuttleI: boolean, count: number }src/app/data/routemap/circleanimate.ts (1)
3-7: Type definition looks good but consider adding documentation.The
CircleAnimatetype is well-structured for managing circle animations in a React component. Consider adding JSDoc comments to document the purpose of each property for better maintainability.+/** + * Type definition for managing circle animations in route map visualization + */ export type CircleAnimate = { + /** Reference to array of HTML div elements to be animated */ ref: React.MutableRefObject<Array<HTMLDivElement>>, + /** Index for tracking the animation sequence */ index: number, + /** Color of the chip/circle being animated */ chipColor: string, }src/app/components/lang/lang.ko.json (1)
24-24: Minor formatting change.A space was added after the value of "residence". While this doesn't affect functionality, ensure formatting is consistent across all entries.
src/app/reducer/store.ts (1)
5-8: Action type could be more specificThe current Action interface could be improved using Redux Toolkit's
PayloadActiontype for better type safety.-interface Action{ - type: string, - payload: SingleShuttleSchedule -} +import { PayloadAction } from '@reduxjs/toolkit'; + // Then in the reducer: -updateActions: (state, action: Action) +updateActions: (state, action: PayloadAction<SingleShuttleSchedule>)src/App.tsx (3)
121-123: Check for conflicting height definitions.
Lines 122 and 123 define multiple heights (h-12vs.h-48). The final style should be correct, but confirm the initial rendered height is intended.
199-199: New state hook for route card click.
Storing route expansion inrouteCardClickis appropriate. Consider resetting it when unmounting if relevant.
448-478: Well-structured transition block.
Using<Transition>to animateRouteIndexCardViewprovides a neat user experience. If you plan to handle keyboard accessibility, ensure that arrow keys or tabbing also collapse/expand the card if necessary.src/app/components/fulltime/FullTime.tsx (3)
126-129: Commented-out code preservation.
This entire block is commented out. Remove it if no longer needed to maintain code cleanliness, or add a TODO note clarifying future usage.
301-301: Confirm placeholder return.
Line 299'sreturn <></>may be an accidental leftover from debugging. Consider removing or clarifying its purpose.
311-311: Ensure max chip count is updated only when needed.
Re-check how oftensetCountChip(maxCount)runs to avoid unnecessary re-renders..pnp.cjs (4)
3363-3396: Enhanced @reduxjs/toolkit Dependency Block
A comprehensive block for@reduxjs/toolkithas been added, including both direct (npm:2.5.0) and virtual entries that bundle additional peer dependencies (e.g.,immer,react-redux, etc.). Please ensure that the mix of soft and hard linking is intentional and that all included dependencies are compatible.
4154-4169: New Type Definitions: @types/react-transition-group
The dependency block for@types/react-transition-group(version 4.4.11) is now in place with its peer dependency on@types/react. Verify that this version aligns with your usage in any animated transition components. The subsequent block for@types/resolvealso appears standard.
5835-5851: DOM Helpers Dependency Added
A new block fordom-helpers(version 5.2.1) has been added with its dependencies (including@babel/runtimeandcsstype). This should support improved DOM utilities. Verify its compatibility with any UI components (like the new route map) that might manipulate DOM elements.
9874-9888: Reselect & Resolve Patch
A dependency block forreselect(version 5.1.1) has been added alongside a patched declaration forresolve. The patch notation forresolveshould be reviewed to ensure it is well documented and its impact is fully understood.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (13)
.yarn/cache/@babel-runtime-npm-7.26.0-9afa3c4ef6-9f4ea1c1d5.zipis excluded by!**/.yarn/**,!**/*.zip.yarn/cache/@reduxjs-toolkit-npm-2.5.0-9bdc99574d-b24ee7e89b.zipis excluded by!**/.yarn/**,!**/*.zip.yarn/cache/@types-react-transition-group-npm-4.4.11-dda183055d-a7f4de6e5f.zipis excluded by!**/.yarn/**,!**/*.zip.yarn/cache/@types-use-sync-external-store-npm-0.0.6-9e5c635381-a95ce33066.zipis excluded by!**/.yarn/**,!**/*.zip.yarn/cache/dom-helpers-npm-5.2.1-b38bb4470b-bed2341adf.zipis excluded by!**/.yarn/**,!**/*.zip.yarn/cache/immer-npm-10.1.1-973ae10d09-9dacf1e8c2.zipis excluded by!**/.yarn/**,!**/*.zip.yarn/cache/react-redux-npm-9.2.0-d87bb27c82-b3d2f89f46.zipis excluded by!**/.yarn/**,!**/*.zip.yarn/cache/react-transition-group-npm-4.4.5-98ea4ef96e-ca32d3fd21.zipis excluded by!**/.yarn/**,!**/*.zip.yarn/cache/redux-npm-5.0.1-f8e6b1cb23-a373f9ed65.zipis excluded by!**/.yarn/**,!**/*.zip.yarn/cache/redux-thunk-npm-3.1.0-6a8fdd3211-38c563db5f.zipis excluded by!**/.yarn/**,!**/*.zip.yarn/cache/reselect-npm-5.1.1-667568f51c-1fdae11a39.zipis excluded by!**/.yarn/**,!**/*.zip.yarn/cache/use-sync-external-store-npm-1.4.0-176448bea1-08bf581a8a.zipis excluded by!**/.yarn/**,!**/*.zipyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (17)
.pnp.cjs(15 hunks)package.json(3 hunks)src/App.tsx(4 hunks)src/app/components/fulltime/FullTime.tsx(7 hunks)src/app/components/index.ts(1 hunks)src/app/components/lang/lang.en.json(2 hunks)src/app/components/lang/lang.ko.json(2 hunks)src/app/components/routemap/RouteMap.tsx(1 hunks)src/app/components/shuttle/Shuttle.tsx(4 hunks)src/app/data/index.ts(1 hunks)src/app/data/routemap/circleanimate.ts(1 hunks)src/app/data/shuttle/organizedTimetables.ts(1 hunks)src/app/network/shuttle.ts(1 hunks)src/app/reducer/store.ts(1 hunks)src/index.css(4 hunks)src/main.tsx(2 hunks)tailwind.config.js(4 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
src/app/components/fulltime/FullTime.tsx
[error] 299-299: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Generate Lighthouse Report
- GitHub Check: Cloudflare Pages
🔇 Additional comments (45)
src/app/components/index.ts (1)
2-2: This file change appears unnecessary.This export statement modification doesn't add any functional changes to the codebase, as it's exporting the same component both before and after the change.
src/app/data/index.ts (1)
5-5: LGTM - New type export for route map functionality.The addition of the
CircleAnimatetype export supports the new route map feature being implemented in this PR.src/app/data/routemap/circleanimate.ts (1)
1-7: The file name should follow lowerCamelCase convention.The file should be renamed from
circleanimate.tstocircleAnimate.tsto follow the lowerCamelCase naming convention used in React projects.src/main.tsx (3)
8-8: Redux integration looks good.The addition of Redux through the Provider component is implemented correctly. The imports for Provider and store are properly placed.
Also applies to: 13-13
45-45: Why is this z-index changed?The z-index was changed from 1 to 12 without clear justification. This may affect the layering of UI elements and could potentially cause issues with other components. Please explain the reason for this change.
#!/bin/bash # Check for other z-index values in the codebase to understand the context echo "Searching for z-index declarations in the codebase..." rg -A 2 -B 2 "zIndex:" --type=tsx --type=jsx --type=css
51-53: Redux Provider is correctly implemented.The Redux Provider wrapping around the App component is correctly placed in the component hierarchy, allowing all child components to access the Redux store.
src/app/components/lang/lang.ko.json (1)
12-13: New localization entries look good.The added localization entries for "yesul", "jung", and "dorm" are consistent with the English translations. These additions will support the new RouteMap feature.
Also applies to: 14-14, 25-25
src/app/components/lang/lang.en.json (2)
12-14: New localization entries match Korean translations.The added entries for "yesul" and "jung" correspond well with their Korean counterparts, maintaining localization consistency.
25-25: Consistent "dorm" entry added.The "dorm" entry has been properly added to match the Korean translation, ensuring feature parity across languages.
src/index.css (1)
31-31: New color variable addition looks good!The new CSS variable
--color-arrow-colorhas been consistently added to all theme variations with appropriate color values for each theme context.Also applies to: 55-55, 79-79, 103-103
package.json (1)
15-15: New dependencies for state management and transitions look appropriateThe addition of Redux-related packages (
@reduxjs/toolkit,react-redux,redux) and the transition group library will enable centralized state management and animated UI transitions.However, per previous discussion, please clarify why Redux is needed alongside the existing useState-based state management.
Also applies to: 30-30, 36-36, 38-38, 47-47
src/app/reducer/store.ts (2)
10-23: Optimize state clearing logicThe current approach to clearing the state array using a while loop is inefficient. Redux Toolkit's
createSliceuses Immer under the hood, which allows for simpler state mutations.updateActions: (state, action: Action) => { - while(state.length > 0){ - state.pop() - } + state.length = 0; // This is more efficient for clearing an array if(action.payload !== undefined){ state.push(action.payload) } }
25-29: Store configured correctlyThe Redux store is properly configured with the actions reducer.
src/app/components/shuttle/Shuttle.tsx (2)
340-342: Good UI enhancement for different shuttle typesAdding the orange background color for 'R' and 'NA' shuttle types improves visual differentiation in the UI.
349-349: Redux dispatch initialization looks goodThe dispatch function is correctly initialized using the useDispatch hook.
tailwind.config.js (4)
47-47: New color variable usage looks good.
Defining'chip-orange': '#FFAB40'aligns with typical Material Orange 400. No issues found.
146-148: Verify transition property usage.
Thetransition-[height]utility is already declared in your component, so adding"height": "height"is consistent. Confirm all cross-browser behaviors if needed.
14-15:❓ Verification inconclusive
Consider consistent naming across new breakpoints.
These min breakpoints (mwideat 430px andwideat 455px) are fairly close together. Verify they don't clash with existing breakpoints or cause overlapping layout issues.Would you like me to scan the codebase for references to these breakpoints to confirm their usage and styling impact?
🏁 Script executed:
#!/bin/bash fd --extension js --extension ts --extension tsx --exec rg --context 5 'mwide|wide'Length of output: 962
Breakpoint Naming & Overlap Consistency Verification
I scanned the codebase and confirmed that both
wide(min: 455px) andmwide(min: 430px) are referenced in the Tailwind configuration as well as in the styled components (e.g., inRouteTextContainer). Their close proximity in value appears intentional. However, please manually review the layout behavior to ensure that the 25px difference does not cause any overlapping or unintended styling issues. Also, consider if the naming convention aligns well with the rest of our breakpoints for consistency.
35-35:✅ Verification successful
Ensure CSS variable is defined.
Please confirm that--color-arrow-coloris declared in your CSS (e.g.,:rootor theme classes).
🏁 Script executed:
#!/bin/bash rg --context 3 'color-arrow-color'Length of output: 2303
CSS Variable Definition Verified – No Further Action Required
The variable--color-arrow-coloris confirmed to be declared insrc/index.css(with multiple definitions for different theme contexts) and is correctly referenced in the Tailwind configuration. Please ensure that the intended theme definition is applied in the expected context.src/App.tsx (3)
1-1: Imports look consistent.
The newly added imports forTransition,Arrow, andRouteMapare valid. Ensure that the relevant packages are correctly installed (e.g.,react-transition-group).Also applies to: 5-5, 10-10, 14-14
130-134: Verify container visibility logic.
Hiding the container whenstatusis'entered'may be counterintuitive. Double-check that this aligns with your UI expectations, as the container becomeshiddenafter the transition completes.
135-138: Rotation toggle is straightforward.
Rotating theRouteToggleImagebased onenteredvs.exitedis a clean approach.src/app/components/fulltime/FullTime.tsx (9)
103-104: Extended layout height.
The new$maxChips === 4condition ensures enough space for additional timetable rows. No concerns here.
180-183: Additional “directY” grid logic.
Introducing a separate grid forprops.directYclarifies the distinction from regulardirecttimes. Keep in mind to test UI for each route type thoroughly.
185-196: Conditional shuttle color and direct mapping.
• Good approach usingprops.isShuttleI ? "bg-chip-orange" : "bg-chip-blue".
• The mapping that checks for duplicates indirectYis correct, but ensure no edge cases exist (e.g., partial string matches).
262-263: Initialization withisShuttleI.
isShuttleI: falseis a sensible default. Confirm that no existing data or code paths rely on a different default.
279-280: New logic forDHJorRschedules.
Handlingschedule.type === 'R'to setisShuttleI = trueis consistent. Just verify that other code referencesisShuttleIin relevant conditions.
293-295: Counting directY items.
Incrementingsingle.countifdirectYis non-empty aligns with the rest of your logic. No issues observed.
317-317: Check rendering condition.
Line 317 filters out times with zero direct/circle/directY/jungang only. This is correct, but watch for a scenario that might skip data inadvertently.
325-325: EnsureisShuttleIis passed.
Props are destructured properly, but confirm thatTimeBoxalways receivesisShuttleIfrom the parent.
331-331: Grid container usage.
The timetable display wraps with a.grid grid-flow-rowpattern. Looks fine; no immediate concerns.src/app/components/routemap/RouteMap.tsx (3)
49-51: Check transition states.
Applyingopacity-100on'entered'or'exit'can be confusing. Ensure the transition callback states are spelled correctly (exiting,exited,entering,entered).
239-251: Double append ofpingCircle.
Similar to the previous review, line 249 appendspingCircleagain after lines 245-248, leading to duplicates.- props.ref.current[i]?.append(pingCircle)Remove or conditionally guard the second append to avoid duplicating the element.
273-281: Consistent route type logic.
Mapping eachtypeto the correct ref and color is clean. No immediate issues..pnp.cjs (11)
27-38: New Dependency Addition: @reduxjs/toolkit & @types/react-transition-group
A new entry for@reduxjs/toolkit(version 2.5.0) and the corresponding type definitions forreact-transition-grouphave been added. Please verify that the virtual dependency specifications match those in yourpackage.jsonand that these versions meet your project’s requirements.
59-73: React-Redux & Related Dependency Updates
The dependency list now includesreact-redux(version 9.2.0) along with several other react-related packages. Ensure that these virtual package declarations are consistent across your configuration and align with your project’s state management strategy.
2732-2745: @babel/runtime Configuration Update
A new configuration block for@babel/runtime(version 7.26.0) has been introduced with its dependencies (includingregenerator-runtime). Confirm that these versions are compatible with your Babel setup and that there’s no version mismatch with other tooling.
4212-4226: TypeScript Linting & use-sync-external-store Typings
The configuration now includes@types/use-sync-external-store(version 0.0.6), along with an update to the@typescript-eslint/eslint-plugin. Please confirm that these versions support your linting and type-checking configurations without conflicts.
7267-7278: Workspace Dependency Consistency Check
This section reiterates dependency definitions for the workspace, notably reintroducing@reduxjs/toolkitwith its virtual package hash. Ensure that these declarations match those in other parts of your configuration to avoid version conflicts.
7299-7313: React Dependencies Consistency
The block repeats several react-related dependencies (e.g.,react-redux,react-router-dom,react-transition-group, etc.) with updated virtual entries. Verify that these versions remain consistent across all dependency sections and align with your application's expected behavior.
7396-7410: State Management Libraries: Immer vs. Immutable
A new dependency block forimmer(version 10.1.1) has been added immediately before theimmutableentry. Given the overlapping functionality of these libraries, please reassess whether you need both in your codebase. (See previous review comments on potential overlap.)
9437-9470: React-Redux Dependency Block Review
Another block forreact-redux(version 9.2.0) is present with both npm and virtual entries. Ensure that its duplication is intentional and that it does not introduce conflicts with earlier declarations in the file.
9626-9661: React-Transition-Group Configuration Refinement
The dependency block forreact-transition-groupnow features both npm and virtual entries (with additional dependencies like@babel/runtimeand peer types). Please confirm that these extra peer dependencies are required and that the soft/hard linking strategy is correct.
9733-9769: Redux & Redux-Thunk Dependency Updates
The file now includes updated blocks forredux(version 5.0.1) andredux-thunk(version 3.1.0), with redux-thunk having both npm and virtual entries. Verify that these versions and linking types ensure a smooth Redux middleware integration.
11334-11361: use-sync-external-store & util-deprecate Configuration
This block sets upuse-sync-external-store(version 1.4.0) with both npm and virtual entries and then continues with theutil-deprecateconfiguration. Confirm that these versions are necessary and that their inclusion benefits your state synchronization strategy.
Describe your Pull Request
Please describe the behavior or changes of your code specifically.
Additional content
Any additional comments? 😁
Summary by CodeRabbit
New Features
Style