Add Route Map #99
Add Route Map #99
Conversation
…te Shuttlecoke_i color
WalkthroughThis pull request introduces several dependency updates and new components across the codebase. There are additions in package management, including new libraries and type definitions, along with enhancements to animations and transitions in the UI. New React components for route visualization and dot animations have been implemented, and state management has been improved through a newly introduced TimeTableContext. Additionally, multiple style, formatting, and localization adjustments have been made to enhance consistency across files. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant RouteMap
participant RouteVisual
participant DotAnimation
participant useDotAnimation
User->>RouteMap: Selects or navigates to a route
RouteMap->>RouteVisual: Renders route details with status and tab info
RouteVisual->>useDotAnimation: Request animation configurations
useDotAnimation-->>RouteVisual: Returns dot animation flags/config
RouteVisual->>DotAnimation: Render animated dots based on flags
DotAnimation--)RouteVisual: Display animation results
sequenceDiagram
participant MainApp
participant TimeTableContextProvider
participant Shuttle
participant TimeTableContext
MainApp->>TimeTableContextProvider: Wrap App with context
Shuttle->>TimeTableContext: Access timetable state via hook
Shuttle->>TimeTableContext: Call setTimetable() with new or default data
TimeTableContext-->>Shuttle: Return updated timetable state
Poem
✨ Finishing Touches
🪧 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: |
ceb7995
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://21729f07.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.
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.
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]))
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.
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.
There was a problem hiding this comment.
Actionable comments posted: 5
🔭 Outside diff range comments (1)
src/app/components/subway/Subway.tsx (1)
291-297:⚠️ Potential issueCritical Typo in Window Target
In the
openApiMonitorfunction, the target parameter is set to'_black', which is almost certainly a typo. The correct target should be'_blank'in order to open the URL in a new browser tab.Apply the following diff:
- '_black', + '_blank',
♻️ Duplicate comments (2)
src/app/data/shuttle/organizedTimetables.ts (1)
7-7:⚠️ Potential issueNew property needs proper initialization in all usage locations.
Adding the
isShuttleIproperty to theOrganizedTimetablestype is valid, but as noted in a previous review, ensure this property is properly initialized in all places whereOrganizedTimetablesobjects are created.For example, in
src/app/components/fulltime/FullTime.tsx, the creation ofOrganizedTimetablesobjects needs to be updated to include this new property:const single: OrganizedTimetables = { time: hour, direct: [], + isShuttleI: false, // Add initialization for the new property }Please verify all instances where
OrganizedTimetablesobjects are created to avoid runtime errors.src/app/components/shuttle/Shuttle.tsx (1)
423-430:⚠️ Potential issueAdd error handling for empty filtered bus data.
The current implementation doesn't properly handle the case when
timetable.datais undefined, which could lead to runtime errors.Apply this fix:
useEffect(() => { - if (timetable.data !== undefined) { + if (timetable.data && timetable.data.length > 0) { const filtered = timetable.data.filter((val) => isAfterCurrentTime(val)) filtered[0] === undefined ? setTimetable({ time: '', type: 'NA' }) : setTimetable(filtered[0]) } }, [timetable.data, setTimetable])
🧹 Nitpick comments (11)
.pnp.cjs (1)
2729-2742: New @babel/runtime Dependency BlockA new dependency block for
@babel/runtime(version7.26.0) has been introduced, including its dependency onregenerator-runtime. Please double-check that this block is intentional and that the versions for both@babel/runtimeandregenerator-runtimeare compatible with the rest of your build configuration.src/app/data/routemap/circleAnimate.ts (1)
1-4: Type definition is valid, but documentation would improve clarity.The CircleAnimate type definition looks good. For better maintainability, consider adding JSDoc comments to document the purpose of this type and its properties.
+/** + * Type representing an animated circle element on the route map + */ export type CircleAnimate = { + /** The type of dot/point (e.g., 'station', 'stop', etc.) */ dotType: string + /** Position index of the animation element */ index: number }If
dotTypeis meant to be limited to specific values, consider using a union type instead:dotType: 'station' | 'stop' | 'current' // example valuessrc/app/context/ThemeContext.tsx (1)
26-30: Refactor: Cache Local Storage Value for Theme
The new one-line assignment forthemeNameis concise and maintains the original logic. However, callingwindow.localStorage.getItem('theme')twice can be inefficient and makes the code less DRY. Consider storing the retrieved value in a variable first and then performing the check.src/app/components/modal/modalOpen.tsx (1)
63-77: JSX Mapping Cleanup & Redundant Fragment
The changes to the conditional rendering forprops.mTarget === 'Fabs'improve the code’s compactness. However, the use of?? <></>appears redundant since an empty fragment is not necessary when no content is rendered. Consider returningnullinstead for clarity and to avoid a superfluous fragment.🧰 Tools
🪛 Biome (1.9.4)
[error] 76-76: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
(lint/complexity/noUselessFragments)
src/App.tsx (1)
424-426: Hardcoded$activeIndexvalue.
Using117for$activeIndexmay feel arbitrary. If more modes are added or the layout changes, that constant could break. Consider a more dynamic approach.src/app/components/routemap/RouteVisual.tsx (1)
174-612: Repeated dot rendering logic.
The functionsdirectDot,cycleDot,yesulDot, andjungDotshare similar loops and patterns. Consider a generic helper that builds dot arrays based on route properties to reduce code duplication.src/index.css (1)
7-14: Consider browser-specific property management.Several browser-specific properties are used for preventing user selection, but there are issues with a couple of them:
- The
-khtml-user-selectproperty is deprecated and unnecessary in modern browsers- The
-user-dragproperty is non-standard and should be-webkit-user-draginsteadbody { -webkit-touch-callout: none; -webkit-user-select: none; -webkit-user-drag: none; - -khtml-user-select: none; -moz-user-select: none; -ms-user-select: none; user-select: none; - -user-drag: none; }🧰 Tools
🪛 Biome (1.9.4)
[error] 10-10: Unknown property is not allowed.
See CSS Specifications and browser specific properties for more details.
To resolve this issue, replace the unknown property with a valid CSS property.(lint/correctness/noUnknownProperty)
[error] 14-14: Unknown property is not allowed.
See CSS Specifications and browser specific properties for more details.
To resolve this issue, replace the unknown property with a valid CSS property.(lint/correctness/noUnknownProperty)
src/app/components/routemap/Animation.tsx (4)
13-48: Consider refactoring circleAnimation with a more elegant approach.The current implementation has repetitive code with multiple if-else statements. This could be simplified using an object property accessor pattern.
export const circleAnimation = ( type: CircleAnimate, setTableArray: React.Dispatch< React.SetStateAction<{ direct: boolean[] cycle: boolean[] yesulin: boolean[] jungang: boolean[] }> >, tabArray: { direct: boolean[] cycle: boolean[] yesulin: boolean[] jungang: boolean[] }, ) => { const newTableArray = { direct: [...tabArray.direct], cycle: [...tabArray.cycle], yesulin: [...tabArray.yesulin], jungang: [...tabArray.jungang], } for (let i = type.index; i <= type.index + 1; i++) { - if (type.dotType === 'direct') { - newTableArray.direct[i] = true - } else if (type.dotType === 'cycle') { - newTableArray.cycle[i] = true - } else if (type.dotType === 'yesulin') { - newTableArray.yesulin[i] = true - } else if (type.dotType === 'jungang') { - newTableArray.jungang[i] = true - } + const validTypes = ['direct', 'cycle', 'yesulin', 'jungang']; + if (validTypes.includes(type.dotType)) { + newTableArray[type.dotType as keyof typeof newTableArray][i] = true; + } } setTableArray(newTableArray) }
50-55: Simplify the timetableType function using an object map.The current implementation uses if-else statements for mapping, which could be improved with a lookup object for better readability and maintainability.
const timetableType = (type: string, index: number) => { - if (type === 'C') return { dotType: 'cycle', index: index } - else if (type === 'DHJ') return { dotType: 'jungang', index: index } - else if (type === 'DY') return { dotType: 'yesulin', index: index } - else return { dotType: 'direct', index: index } + const typeMap: Record<string, string> = { + 'C': 'cycle', + 'DHJ': 'jungang', + 'DY': 'yesulin' + }; + + return { dotType: typeMap[type] || 'direct', index }; }
119-121: Combine useEffect hooks and consider dependency array optimization.There are two separate useEffect hooks that could be combined, and the dependency array in the second hook might cause unnecessary re-renders.
- React.useEffect(() => { - checkTable.current = undefined - }, []) - - useEffect(() => { + React.useEffect(() => { + // Only reset checkTable.current when component mounts + if (!checkTable.current) { + checkTable.current = undefined + } + const tableArray = { direct: [false, false, false, false, false], cycle: [false, false, false, false, false, false], yesulin: [false, false, false, false, false], jungang: [false, false, false, false, false, false], } setTimetable(tableArray) if (checkTable.current === timetables) return checkTable.current = timetables const animateType = CircleAnimateType(tab, timetables) animateType.forEach((item) => { circleAnimation(item, setTimetable, tableArray) }) }, [timetables, tab])Also applies to: 123-142
147-172: Simplify Animation component with a more declarative approach.The component has nested conditional logic that can be simplified using a more declarative approach with object mapping.
const Animation = (props: { isOn: boolean index: number rootStatus: string }) => { - switch (props.rootStatus) { - case 'direct': - if (props.index === 4) - return <PingCircle className="bg-chip-orange" $on={props.isOn} /> - return <PingCircle className="bg-chip-blue" $on={props.isOn} /> - case 'cycle': - if (props.index === 5) - return <PingCircle className="bg-chip-orange" $on={props.isOn} /> - return <PingCircle className="bg-chip-red" $on={props.isOn} /> - case 'yesulin': - if (props.index === 4) - return <PingCircle className="bg-chip-orange" $on={props.isOn} /> - return <PingCircle className="bg-chip-green" $on={props.isOn} /> - case 'jungang': - if (props.index === 5) - return <PingCircle className="bg-chip-orange" $on={props.isOn} /> - return <PingCircle className="bg-chip-purple" $on={props.isOn} /> - default: - return <></> - } + const { isOn, index, rootStatus } = props; + + // Define the special indices where orange should be used + const orangeIndices = { + direct: 4, + cycle: 5, + yesulin: 4, + jungang: 5 + }; + + // Define the default color for each status + const defaultColors = { + direct: "bg-chip-blue", + cycle: "bg-chip-red", + yesulin: "bg-chip-green", + jungang: "bg-chip-purple" + }; + + // If rootStatus is not in our mappings, return empty fragment + if (!defaultColors[rootStatus as keyof typeof defaultColors]) { + return <></>; + } + + // Determine if this should be orange based on index + const useOrange = index === orangeIndices[rootStatus as keyof typeof orangeIndices]; + const className = useOrange ? "bg-chip-orange" : defaultColors[rootStatus as keyof typeof defaultColors]; + + return <PingCircle className={className} $on={isOn} />; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (5)
.yarn/cache/@babel-runtime-npm-7.26.0-9afa3c4ef6-9f4ea1c1d5.zipis excluded by!**/.yarn/**,!**/*.zip.yarn/cache/@types-react-transition-group-npm-4.4.11-dda183055d-a7f4de6e5f.zipis excluded by!**/.yarn/**,!**/*.zip.yarn/cache/dom-helpers-npm-5.2.1-b38bb4470b-bed2341adf.zipis excluded by!**/.yarn/**,!**/*.zip.yarn/cache/react-transition-group-npm-4.4.5-98ea4ef96e-ca32d3fd21.zipis excluded by!**/.yarn/**,!**/*.zipyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (30)
.pnp.cjs(8 hunks)package.json(3 hunks)src/App.tsx(11 hunks)src/app/components/fab/fab.scss(0 hunks)src/app/components/fab/fab.tsx(1 hunks)src/app/components/fulltime/FullTime.tsx(8 hunks)src/app/components/lang/i18n.ts(1 hunks)src/app/components/lang/lang.en.json(1 hunks)src/app/components/lang/lang.ko.json(1 hunks)src/app/components/modal/modalOpen.tsx(4 hunks)src/app/components/ptr/refreshing-content.tsx(1 hunks)src/app/components/routemap/Animation.tsx(1 hunks)src/app/components/routemap/Responsive.tsx(1 hunks)src/app/components/routemap/RouteMap.tsx(1 hunks)src/app/components/routemap/RouteVisual.tsx(1 hunks)src/app/components/shuttle/Shuttle.tsx(6 hunks)src/app/components/subway/Subway.tsx(1 hunks)src/app/components/useDarkMode.ts(3 hunks)src/app/context/ThemeContext.tsx(1 hunks)src/app/context/TimeTableContext.tsx(1 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/apiHandler.ts(1 hunks)src/app/network/shuttle.ts(1 hunks)src/app/network/subway.ts(1 hunks)src/index.css(1 hunks)src/main.tsx(2 hunks)src/vite-env.d.ts(1 hunks)tailwind.config.js(4 hunks)
💤 Files with no reviewable changes (1)
- src/app/components/fab/fab.scss
🧰 Additional context used
🪛 Biome (1.9.4)
src/app/components/modal/modalOpen.tsx
[error] 76-76: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
(lint/complexity/noUselessFragments)
src/index.css
[error] 10-10: Unknown property is not allowed.
See CSS Specifications and browser specific properties for more details.
To resolve this issue, replace the unknown property with a valid CSS property.
(lint/correctness/noUnknownProperty)
[error] 14-14: Unknown property is not allowed.
See CSS Specifications and browser specific properties for more details.
To resolve this issue, replace the unknown property with a valid CSS property.
(lint/correctness/noUnknownProperty)
[error] 24-24: Unexpected unknown function: theme
Use a known function instead.
See MDN web docs for more details.
(lint/correctness/noUnknownFunction)
[error] 25-25: Unexpected unknown function: theme
Use a known function instead.
See MDN web docs for more details.
(lint/correctness/noUnknownFunction)
[error] 26-26: Unexpected unknown function: theme
Use a known function instead.
See MDN web docs for more details.
(lint/correctness/noUnknownFunction)
[error] 28-28: Unexpected unknown function: theme
Use a known function instead.
See MDN web docs for more details.
(lint/correctness/noUnknownFunction)
[error] 29-29: Unexpected unknown function: theme
Use a known function instead.
See MDN web docs for more details.
(lint/correctness/noUnknownFunction)
[error] 43-43: Unexpected unknown function: theme
Use a known function instead.
See MDN web docs for more details.
(lint/correctness/noUnknownFunction)
[error] 44-44: Unexpected unknown function: theme
Use a known function instead.
See MDN web docs for more details.
(lint/correctness/noUnknownFunction)
[error] 48-48: Unexpected unknown function: theme
Use a known function instead.
See MDN web docs for more details.
(lint/correctness/noUnknownFunction)
[error] 49-49: Unexpected unknown function: theme
Use a known function instead.
See MDN web docs for more details.
(lint/correctness/noUnknownFunction)
[error] 50-50: Unexpected unknown function: theme
Use a known function instead.
See MDN web docs for more details.
(lint/correctness/noUnknownFunction)
[error] 52-52: Unexpected unknown function: theme
Use a known function instead.
See MDN web docs for more details.
(lint/correctness/noUnknownFunction)
[error] 76-76: Unexpected unknown function: theme
Use a known function instead.
See MDN web docs for more details.
(lint/correctness/noUnknownFunction)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Generate Lighthouse Report
- GitHub Check: Cloudflare Pages
🔇 Additional comments (54)
.pnp.cjs (7)
31-37: Added Type Definitions for React Transition GroupThe dependency entry for
@types/react-transition-groupat version4.4.11has been added here. Please ensure it aligns with the type requirements of your code that usesreact-transition-group, and that you have updated any relevant imports or usages accordingly.
63-69: Updated React Transition Group Dependency EntryThis hunk updates the
react-transition-groupdependency to use version4.4.5via a virtual package. Verify that this update is consistently applied in your dependency graph and that the virtual resolution strategy does not introduce conflicts with other package versions.
4121-4134: Added Dependency Block for @types/react-transition-groupThis block explicitly defines the resolution for
@types/react-transition-group(version4.4.11) along with its dependency on@types/react. Confirm that this entry does not conflict with similar listings elsewhere in the file and that it supports all usage scenarios in your application.
5793-5809: Added Dependency Block for dom-helpersThe new entry for
dom-helpersat version5.2.1(with dependencies on@babel/runtimeandcsstype) appears correctly set up. Please ensure that this addition integrates well with your runtime and that any features relying on this package have been tested.
7229-7235: Verify Duplicate Entry for @types/react-transition-groupThis section repeats the dependency entry for
@types/react-transition-groupat version4.4.11. Please verify that having duplicate entries in different parts of the file is intentional (for instance, if they serve different resolution scopes) and does not cause conflicts.
7261-7267: Verify Duplicate React-Transition-Group EntryHere we see another occurrence of the
react-transition-groupdependency (version4.4.5). Confirm that the repetition across different sections is expected within your Yarn Plug’n’Play configuration and that no unintended version mismatches occur.
9544-9579:❓ Verification inconclusive
Complex Dependency Block for React Transition Group
This hunk introduces a detailed dependency block for
react-transition-groupwith two entries—one using a "SOFT" link type and a second with a "HARD" link type. This configuration also specifies extensive package dependencies and peer requirements. Please confirm that this dual setup is intentional and that both the soft and hard link resolutions work as expected in your environment.
Clarify Dual Link Configuration for React Transition Group
The dependency block for
react-transition-groupnow declares two entries:
- One entry uses a "SOFT" link type, and
- Another entry, defined as a virtual package, uses a "HARD" link type.
Please verify that this dual setup is intentional and that your environment properly distinguishes and resolves both the soft and hard links as expected. If these configurations serve different purposes (e.g., supporting both standard and virtual resolutions), a brief note in the documentation or a code comment might help clarify the rationale. Otherwise, consider refactoring to remove any redundancy.
src/app/data/index.ts (1)
5-5: Export statement properly references the new CircleAnimate type.The addition of this export statement correctly makes the CircleAnimate type available throughout the application and resolves the previous build error.
src/app/components/ptr/refreshing-content.tsx (1)
13-13: Whitespace formatting improvement.The removal of an extra blank line helps maintain consistent code formatting.
src/vite-env.d.ts (1)
1-3: No-Op Formatting Change in Type DefinitionsThe re-addition of the Vite client reference (
/// <reference types="vite/client" />) appears to be a formatting or reordering change only, with no impact on type resolution or functionality.src/app/components/subway/Subway.tsx (1)
424-432: JSX Attribute Quotes StandardizationThe
SyncLoadercomponent’scolorprop now uses double quotes ("var(--color-load-color)"), which aligns with common JSX style conventions. This change is purely stylistic.src/app/network/subway.ts (1)
4-6: Trailing Comma for Parameter ListThe addition of a trailing comma after the
station: stringparameter insubwayAPIis a stylistic improvement that can help maintain consistency in parameter declarations. There is no change in functionality.src/app/components/lang/lang.en.json (2)
1-6: JSON Formatting ImprovementThe file’s reformatting from a four-space to a two-space indentation style (with an added newline at the end) improves readability and aligns with common JSON styling guidelines. No keys or values have been altered.
23-27: Translation Consistency: "Dorm" vs "Dormitory"The key
"dorm"is now mapped to"Dorm"whereas"residence"and"dest_dorm"are mapped to"Dormitory". This inconsistency might confuse users. Please consider unifying these terms across the UI.src/app/components/lang/i18n.ts (2)
29-30: Consistent Quote Style in i18n ConfigChanging the language option from double quotes to single quotes (i.e.
lng: 'ko') provides consistency with the project's preferred string literal style. This is a non-functional, readability-focused change.
35-38: Added Trailing Comma for Cleaner Object SyntaxA trailing comma has been added in the
detection.cachesarray. This improves version-control diffs and maintains a consistent object notation style.src/app/components/routemap/Responsive.tsx (1)
1-21: Excellent implementation of a custom React hook for responsive design.The
useResponsivehook is well-implemented, following React best practices. You've correctly:
- Used
useStateto track screen width- Implemented
useEffectwith proper cleanup of event listeners- Added an empty dependency array to ensure the effect runs only once
- Exported the hook as the default export
This is much better than the previous implementation mentioned in past review comments, as it properly manages the event listener lifecycle using React's hooks.
src/app/components/fab/fab.tsx (1)
186-192: Improved code formatting for better readability.The multi-line formatting of the
mainButtonStylesobject improves code readability while maintaining the same functionality. This follows modern JavaScript/TypeScript styling conventions.src/main.tsx (3)
12-12: Good addition of TimeTableContextProvider import.The import of the TimeTableContextProvider is correctly defined, which will allow the application to manage and access timetable state through React context.
44-44: Correct adjustment of z-index value for proper UI layering.The z-index was appropriately changed to 2 as discussed in previous review comments, which will position the cherry blossom effect above the Circle component as intended.
50-52: Well-structured context provider implementation.The TimeTableContextProvider is correctly positioned in the component hierarchy, wrapping the App component while being nested inside the DarkmodeContextProvider. This allows the App component to access the timetable context.
src/app/network/shuttle.ts (2)
13-13: Minor style improvement with trailing comma.Adding a trailing comma after the last parameter is a good practice that makes future parameter additions cleaner in git diffs.
16-16:❓ Verification inconclusive
API versioning change introduced.
The API endpoint has been updated to use the
v2prefix, indicating a new version of the API. This is a good practice for API management.Ensure that:
- The v2 API is deployed and available in all environments
- The response structure is compatible with your current implementation
- Changes are documented for the team
🏁 Script executed:
#!/bin/bash # Check if the v2 endpoint exists in your codebase and is referenced consistently echo "Checking for v2 API references in the codebase..." rg -l "\/v2\/timetable\/" --type tsLength of output: 230
Revised Verification: Corrected v2 API Regex Check
The API endpoint update to include the
v2prefix is noted and appears to be a good practice. However, the verification command originally provided encountered a regex escape error. Please ensure that:
- The v2 API endpoint is consistently referenced across the codebase.
- The API is deployed and available in all environments.
- The response structure remains compatible with the current implementation.
- These changes are documented for the entire team.
Below is an updated verification command with corrected regex syntax:
After running this script, please confirm that all intended references to
/v2/timetable/are present. If manual verification is required, include a tag to revisit the check.
🏁 Script executed:
#!/bin/bash echo "Checking for v2 API references in the codebase with corrected regex..." rg -l '/v2/timetable/' --type tsLength of output: 332
V2 API Endpoint Update Confirmed – Verify Deployment and Compatibility
The
/v2/timetable/${season}/${week}/${location}usage insrc/app/network/shuttle.ts(line 16) has been confirmed by our updated regex search. Please ensure that:
- The v2 API endpoint is deployed and accessible across all environments.
- The response structure from the v2 API is fully compatible with the existing implementation.
- These changes are communicated and documented for the team.
package.json (2)
11-12: Script Updates: Prepush and Prettier Script Added
The addition of the"prepush": "yarn run validate"and"prettier": "prettier --write --cache ./src"scripts improves automated code quality checks. Ensure that all team members are aware of these new scripts to keep consistent workflows.
35-46: Dependency Update: New Transition Group Packages Introduced
The inclusion of"react-transition-group": "^4.4.5"in dependencies and"@types/react-transition-group": "^4"in devDependencies is in line with the project's need for animated transitions. Verify that these versions match your compatibility requirements and that there are no conflicts with other UI libraries.src/app/components/modal/modalOpen.tsx (2)
48-49: Formatting Change: Removal of Extraneous Blank Line
The removal of the blank line after thetoggleThemefunction enhances code compactness without altering functionality.
87-89: JSX Formatting: Consistency in Conditional Blocks
The minor adjustments in the conditional rendering forprops.mTarget === 'Christmas'(and similarly for'Spring') streamline the syntax. Ensure that these changes do not affect intended spacing or conditional behavior.src/app/components/useDarkMode.ts (2)
10-13: Formatting Update: Trailing Comma in Parameter List
The addition of a trailing comma for thevalidDayparameter in thesetCookiefunction aligns with modern TypeScript style guidelines and helps reduce diff noise.
25-26: Dependency Array Update in useCallback
The update adding a trailing comma in the dependency array forsetBarStyleis a minor formatting change improving readability. No functional impact noted.src/app/components/lang/lang.ko.json (1)
9-15: Inconsistency: Removal of Key Not Reflected
The AI summary mentioned that the key"cycle_ja"would be removed, yet it remains in the file. Verify whether this key should be removed to avoid duplicate or conflicting translations. Additionally, check the naming consistency between"yesul"and"yesulin"to ensure clarity for translators and developers.Likely an incorrect or invalid review comment.
src/app/components/shuttle/Shuttle.tsx (4)
13-13: Good addition of context hook for state management.Using the
useTimeTableContexthook improves the architecture by allowing timetable data to be shared across components.
353-353: LGTM! Destructuring assignment from context.The destructuring syntax cleanly extracts just the
setTimetablefunction from the context.
339-340: Good extension of ColoredChip component.The addition of 'R' and 'NA' bus types with orange background improves visual consistency and user experience.
565-565: Good use of CSS variables.Replacing hardcoded color values with CSS variables improves theme consistency and supports dark mode adaptability.
Also applies to: 581-581
src/app/context/TimeTableContext.tsx (4)
5-8: Well-designed context interface.The interface clearly defines the shape of the context with appropriate types for the timetable state and setter function.
12-16: Good implementation of custom hook.The
useTimeTableContexthook follows best practices by checking if the context exists and providing a meaningful error message.
26-26: Good use of useMemo for context value.Using
useMemooptimizes performance by preventing unnecessary re-renders when the context value hasn't changed.
18-33:❓ Verification inconclusive
Add null check for context access.
The current implementation initializes timetable with an empty time and 'NA' type, but other components might expect a valid timetable object.
Consider checking if the context is null before accessing its properties in components:
🏁 Script executed:
#!/bin/bash # Check if there are any direct accesses to timetable.time or timetable.type without null checks rg -A 3 "timetable\.(time|type)" --type ts --type tsxLength of output: 83
Context Access Null Check – Verification Needed
The current implementation in
src/app/context/TimeTableContext.tsxsuccessfully provides a non-null default value via the provider. However, as the review comment highlights, there's a potential risk if consumer components access the context without performing a null check, especially if they might accidentally use the context outside the provider’s boundaries.Next Steps for Verification:
Please verify that all consumer components which call
React.useContext(TimeTableContext)either sit within the provider or include appropriate null checks before accessingtimetable.timeortimetable.type.To assist in this, run the revised search command below, which searches both TypeScript (
*.ts) and TSX (*.tsx) files:# #!/bin/bash rg -A 3 "timetable\.(time|type)" -g "*.tsx" -g "*.ts"Given the initial command error (due to unrecognized
tsxfile type), please perform a manual verification or run the updated script. Make sure no component is inadvertently accessing the context without verifying its presence.
Ensure Safe Context Use – Verify Manual Null Checks in Consumers
The current provider implementation correctly initializes a non-null default timetable value; however, it's important to ensure that all consumer components verify that they’re within the provider’s context. Since our initial shell script search failed due to file type recognition issues, please manually confirm that every consumer of
TimeTableContexteither resides within the provider’s boundaries or safely checks for null before accessing properties liketimetable.timeortimetable.type. If any consumer might be used outside of the provider, consider wrapping the context access in a custom hook such as:export const useTimeTableContext = () => { const context = React.useContext(TimeTableContext) if (!context) { throw new Error( "useTimeTableContext must be used within a TimeTableContextProvider." ) } return context }This approach helps prevent runtime errors when the context isn't available.
tailwind.config.js (3)
14-15: Good addition of responsive breakpoints.The new screen sizes enhance responsive design capabilities for wider screens, improving layout flexibility.
35-35: Good addition of color variables.The new color variables ('arrow-color' and 'chip-orange') align with the expanded UI elements and maintain consistent styling.
Also applies to: 47-47
146-148: Good addition of height transition property.Adding the height transition property enables smooth animations for elements that change height dynamically.
src/app/network/apiHandler.ts (1)
13-13:❓ Verification inconclusive
Potential preparation for API enhancements.
Adding trailing commas to type constraints and parameters suggests preparation for future extensions. Ensure this aligns with the versioning changes mentioned in the summary.
Let's check if the API endpoint URLs have been updated to include versions:
Also applies to: 15-15
🏁 Script executed:
#!/bin/bash # Check for API endpoint version changes rg "/v2/timetable/" --type ts --type tsxLength of output: 68
Verify API Endpoint Versioning and TSX File Search
The trailing commas indicate preparation for future API enhancements. However, the initial ripgrep command yielded an error because TSX files weren't recognized. Please manually verify that the API endpoint URLs have been updated to include versioning (e.g.,
/v2/timetable/) in the codebase. Specifically, double-check that any changes intended for lines 13 and 15 insrc/app/network/apiHandler.tsalign with the versioning changes mentioned in the summary.For a more complete search, you might try:
rg "/v2/timetable/" --type ts --type-add 'tsx:*.tsx'src/app/components/fulltime/FullTime.tsx (5)
147-147: Inline fill appearance.
This direct usage offill="var(--color-ft-selected)"is fine, but ensure the CSS variable is defined and updated for all themes. No issues found otherwise.
175-178: New directY block.
The addition ofdirectYwith a green chip for "yesul" is consistent with the code's approach. The usage of.join(' ')to display times is straightforward.
181-184: Conditional chip color for direct route.
Usingprops.isShuttleIto dynamically switch between'bg-chip-orange'and'bg-chip-blue'is a clean approach for highlighting the R-type schedule.
292-293: Incrementing count fordirectY.
This line follows the same pattern used for circle, direct, and jungang arrays. It keeps the code consistent.
315-317: Conditional rendering for empty schedules.
Skipping<TimeBox>rendering when all arrays are empty is a clear approach to avoid showing blank boxes.src/app/components/routemap/RouteMap.tsx (1)
40-78: Overall layout and translation usage look good.
The responsive grid for route rows/columns is well-structured, and translation keys (t('dest_shuttle_o'), etc.) appear consistent. No major issues found.src/App.tsx (3)
127-133: Styled component forRouteIndexCardView.
Adding{ status: string }to handle dynamic heights via transitions is clear. Verify that these heights match your design across all breakpoints.
456-491: Transition blocks for route index.
The approach of using theTransitionstate (state) to control the child components is valid. Ensure'exiting'and'exited'are also handled if you want specific animations.Would you like a verification script to search for all
'exiting'or'exited'references in the codebase?
5-5:❓ Verification inconclusive
Validate installation of
react-transition-group.
You’re importingTransitionfromreact-transition-group. Ensure the package is installed and listed in your dependencies.
🏁 Script executed:
#!/bin/bash rg '"react-transition-group"' package.json npm list react-transition-groupLength of output: 170
Dependency Verification: Check Installation of
react-transition-group
- The dependency is declared in your package.json as
"react-transition-group": "^4.4.5", which confirms that it’s intended to be used.- However, the output from
npm list react-transition-groupreturned an empty tree. This suggests that the package might not have been installed in your node_modules.- Please manually verify that the
node_modules/react-transition-groupdirectory exists. If it’s missing, perform a full installation (e.g., runnpm install) to ensure the dependency is correctly installed.src/app/components/routemap/RouteVisual.tsx (1)
40-41: Conditional early returns on line indices fordirect.
Skipping rendering forprops.index === 4andprops.index === 5might cause confusion if the route data changes. Verify that this logic is intentional.src/index.css (2)
31-31: Good addition of arrow color variables across all themes.The new
--color-arrow-colorvariable has been consistently added to all theme variants, providing a unified way to style arrow elements throughout the application.Also applies to: 55-55, 79-79, 103-103
105-110:❓ Verification inconclusive
Ensure color choices maintain sufficient contrast ratio.
For the Spring theme, several of the colors used (especially the pink shades) should be verified to maintain proper contrast ratios for accessibility, particularly between text colors and their backgrounds.
🌐 Web query:
What is the minimum contrast ratio required for WCAG AA compliance for normal text?💡 Result:
The minimum contrast ratio required for WCAG AA compliance for normal text is 4.5:1. This standard applies to most text under 18pt (24px) or 14pt bold (18.67px)[1][3][7][9].
Key details:
- Large text exception: Text meeting the size thresholds (18pt/24px or 14pt bold/18.67px) only requires 3:1 contrast[2][4][9].
- Enhanced requirements: WCAG AAA level requires 7:1 contrast for normal text and 4.5:1 for large text[5][8].
- Enforcement scope: These rules apply to both web content and mobile applications[3], covering text in UI components, images of text, and graphical elements[12].
Common implementation errors include using incorrect font size units (px vs pt)[4] and assuming brand colors are exempt from requirements[4]. Tools like Accessible Web's contrast checker[1] and WebAIM's analyzer[7] help verify compliance.
Citations:
- 1: https://accessibleweb.com/color-contrast-checker/
- 2: https://www.digitala11y.com/understanding-sc-1-4-3-contrast-minimum/
- 3: https://blog.equally.ai/developer-guide/contrast-minimum/
- 4: https://help.siteimprove.com/support/solutions/articles/80001050538-accessibility-rule-color-contrast-does-not-meet-requirement-explained
- 5: https://meta.discourse.org/t/accessibility-improve-contrast-of-text-styling-in-default-ui/14632
- 6: https://adasitecompliance.com/ada-color-contrast-checker/
- 7: https://webaim.org/articles/contrast/
- 8: https://www.getstark.co/blog/accessible-contrast-ratios-and-a-levels-explained/
- 9: https://www.w3.org/WAI/WCAG21/Understanding/contrast-minimum.html
- 10: https://www.w3.org/TR/UNDERSTANDING-WCAG20/visual-audio-contrast-contrast.html
- 11: https://www.oregon.gov/ode/accessibility/checklist/pages/contrast.aspx
- 12: https://dequeuniversity.com/rules/axe/4.8/color-contrast
Action: Verify Color Contrast Ratios for the Spring Theme
In
src/index.css(lines 105-110), please ensure that the defined color variables meet the WCAG AA requirements. Specifically:
- Minimum Requirement: For normal text, the contrast ratio should be at least 4.5:1.
- Key Pairings to Check:
- The background color (
--color-ft-active: #f16da4) with its corresponding text color (--color-ft-active-text: #ffffff) must reach or exceed the 4.5:1 ratio.- Verify that other combinations, such as
--color-ft-text: #c9255eagainst its intended background, also satisfy this requirement.Using a reliable contrast checker tool (e.g., WebAIM Contrast Checker) is recommended to confirm compliance. Adjust the color values if necessary to meet the required contrast standards.
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
src/app/components/routemap/RouteMap.tsx (1)
28-34:⚠️ Potential issueFix incorrect transition state usage.
The
'exit'state used in the conditional styling is not a standard transition state in react-transition-group.The valid lifecycle states are
'entering','entered','exiting', and'exited'. Please update the condition:- ${(props) => - props.status === 'entered' || props.status === 'exit' - ? tw`opacity-100` - : tw`opacity-0`} + ${(props) => + props.status === 'entered' || props.status === 'entering' + ? tw`opacity-100` + : tw`opacity-0`}
🧹 Nitpick comments (7)
src/app/components/modal/modalOpen.tsx (1)
63-76: Improve the structure of the conditional rendering.The parentheses restructuring around the
changelogs.data?.map()function call provides better code readability. The current format properly ensures the entire expression is treated as a single unit.However, the static analysis tool correctly identified an unnecessary Fragment at line 76. You could simplify the fallback.
{props.mTarget === 'Fabs' && (changelogs.data?.map( (datum: { date: string; details: Array<string> }) => { const arr: string[] = datum.details return ( <ChangelogMargin key={datum.date}> <h4>{datum.date}</h4> {arr.map((item) => ( <P key={item}>{item}</P> ))} </ChangelogMargin> ) }, - ) ?? <></>)} + ) ?? null)}🧰 Tools
🪛 Biome (1.9.4)
[error] 76-76: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
(lint/complexity/noUselessFragments)
src/app/components/fulltime/FullTime.tsx (1)
103-104: Add a fallback for$maxChipsvalues beyond 4.
The logic covers$maxChipsup to 4 but no default. Ensuring a fallback or default case would make your component more robust if future conditions introduce higher chip counts.} else if ($maxChips === 4) { return tw`h-40 hm:h-32` } + else { + return tw`h-auto hm:h-auto` // or any appropriate default }src/index.css (1)
24-29: Tailwindtheme()usage might break standard CSS validation tools.
Functions liketheme('colors.gray.200')aren’t valid in pure CSS. Ensure your build pipeline supports tailwind/twin.macro expansions before finalizing; otherwise, the code may cause unexpected compilation errors.- --color-control-main: theme('colors.gray.200'); - --color-control-active: theme('colors.blue.400'); ... + /* If using Tailwind, consider utility classes or a .css file compiled by PostCSS + Tailwind */ + /* Alternatively, define these as standard hex or reference them dynamically in your tailwind.config.js */Also applies to: 43-52, 76-76
🧰 Tools
🪛 Biome (1.9.4)
[error] 24-24: Unexpected unknown function: theme
Use a known function instead.
See MDN web docs for more details.(lint/correctness/noUnknownFunction)
[error] 25-25: Unexpected unknown function: theme
Use a known function instead.
See MDN web docs for more details.(lint/correctness/noUnknownFunction)
[error] 26-26: Unexpected unknown function: theme
Use a known function instead.
See MDN web docs for more details.(lint/correctness/noUnknownFunction)
[error] 28-28: Unexpected unknown function: theme
Use a known function instead.
See MDN web docs for more details.(lint/correctness/noUnknownFunction)
[error] 29-29: Unexpected unknown function: theme
Use a known function instead.
See MDN web docs for more details.(lint/correctness/noUnknownFunction)
src/app/components/routemap/RouteVisual.tsx (1)
40-41: Avoid abrupt returns for specific indices.
Abrupt returns whenprops.indexis 4 or 5 might be unclear for future maintainers. Consider describing why you're bypassing rendering or rewriting with a named condition for better readability.src/app/components/routemap/Animation.tsx (3)
13-40: Simplify conditional logic in circleAnimation functionThe if-else chain for setting values could be simplified by using the dotType directly as a property name, and the loop logic could be clarified.
export const circleAnimation = ( type: CircleAnimate, tabArray: { direct: boolean[] cycle: boolean[] yesulin: boolean[] jungang: boolean[] }, ) => { const newTableArray = { direct: [...tabArray.direct], cycle: [...tabArray.cycle], yesulin: [...tabArray.yesulin], jungang: [...tabArray.jungang], } - for (let i = type.index; i <= type.index + 1; i++) { - if (type.dotType === 'direct') { - newTableArray.direct[i] = true - } else if (type.dotType === 'cycle') { - newTableArray.cycle[i] = true - } else if (type.dotType === 'yesulin') { - newTableArray.yesulin[i] = true - } else if (type.dotType === 'jungang') { - newTableArray.jungang[i] = true - } - } + // Mark current and next position as active + const currentIndex = type.index; + const nextIndex = type.index + 1; + + if (type.dotType in newTableArray) { + newTableArray[type.dotType][currentIndex] = true; + newTableArray[type.dotType][nextIndex] = true; + } return newTableArray }This approach is more concise, type-safe, and easier to maintain.
42-47: Use TypeScript enums or union types for better type safetyThe
timetableTypefunction maps string types without proper type validation, which could lead to errors if invalid values are passed.+type RouteType = 'C' | 'DHJ' | 'DY' | string; +type DotType = 'cycle' | 'jungang' | 'yesulin' | 'direct'; -const timetableType = (type: string, index: number) => { +const timetableType = (type: RouteType, index: number): { dotType: DotType; index: number } => { if (type === 'C') return { dotType: 'cycle', index: index } else if (type === 'DHJ') return { dotType: 'jungang', index: index } else if (type === 'DY') return { dotType: 'yesulin', index: index } else return { dotType: 'direct', index: index } }This provides better type checking and self-documentation.
140-165: Simplify the Animation component with a more DRY approachThe component has repetitive logic for different status cases. This can be simplified:
const Animation = (props: { isOn: boolean index: number rootStatus: string }) => { + // If no valid rootStatus, return empty fragment + if (!['direct', 'cycle', 'yesulin', 'jungang'].includes(props.rootStatus)) { + return <></>; + } + + // Map rootStatus to colors + const colors = { + direct: 'bg-chip-blue', + cycle: 'bg-chip-red', + yesulin: 'bg-chip-green', + jungang: 'bg-chip-purple' + }; + + // Check if this index should have orange color + const isSpecialIndex = + (props.rootStatus === 'direct' && props.index === 4) || + (props.rootStatus === 'cycle' && props.index === 5) || + (props.rootStatus === 'yesulin' && props.index === 4) || + (props.rootStatus === 'jungang' && props.index === 5); + + const colorClass = isSpecialIndex ? 'bg-chip-orange' : colors[props.rootStatus]; + + return <PingCircle className={colorClass} $on={props.isOn} />; - switch (props.rootStatus) { - case 'direct': - if (props.index === 4) - return <PingCircle className="bg-chip-orange" $on={props.isOn} /> - return <PingCircle className="bg-chip-blue" $on={props.isOn} /> - case 'cycle': - if (props.index === 5) - return <PingCircle className="bg-chip-orange" $on={props.isOn} /> - return <PingCircle className="bg-chip-red" $on={props.isOn} /> - case 'yesulin': - if (props.index === 4) - return <PingCircle className="bg-chip-orange" $on={props.isOn} /> - return <PingCircle className="bg-chip-green" $on={props.isOn} /> - case 'jungang': - if (props.index === 5) - return <PingCircle className="bg-chip-orange" $on={props.isOn} /> - return <PingCircle className="bg-chip-purple" $on={props.isOn} /> - default: - return <></> - } }This approach reduces repetition, making the code more maintainable and easier to understand.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (5)
.yarn/cache/@babel-runtime-npm-7.26.0-9afa3c4ef6-9f4ea1c1d5.zipis excluded by!**/.yarn/**,!**/*.zip.yarn/cache/@types-react-transition-group-npm-4.4.11-dda183055d-a7f4de6e5f.zipis excluded by!**/.yarn/**,!**/*.zip.yarn/cache/dom-helpers-npm-5.2.1-b38bb4470b-bed2341adf.zipis excluded by!**/.yarn/**,!**/*.zip.yarn/cache/react-transition-group-npm-4.4.5-98ea4ef96e-ca32d3fd21.zipis excluded by!**/.yarn/**,!**/*.zipyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (30)
.pnp.cjs(8 hunks)package.json(3 hunks)src/App.tsx(11 hunks)src/app/components/fab/fab.scss(0 hunks)src/app/components/fab/fab.tsx(1 hunks)src/app/components/fulltime/FullTime.tsx(8 hunks)src/app/components/lang/i18n.ts(1 hunks)src/app/components/lang/lang.en.json(1 hunks)src/app/components/lang/lang.ko.json(1 hunks)src/app/components/modal/modalOpen.tsx(4 hunks)src/app/components/ptr/refreshing-content.tsx(1 hunks)src/app/components/routemap/Animation.tsx(1 hunks)src/app/components/routemap/Responsive.tsx(1 hunks)src/app/components/routemap/RouteMap.tsx(1 hunks)src/app/components/routemap/RouteVisual.tsx(1 hunks)src/app/components/shuttle/Shuttle.tsx(6 hunks)src/app/components/subway/Subway.tsx(2 hunks)src/app/components/useDarkMode.ts(3 hunks)src/app/context/ThemeContext.tsx(1 hunks)src/app/context/TimeTableContext.tsx(1 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/apiHandler.ts(1 hunks)src/app/network/shuttle.ts(1 hunks)src/app/network/subway.ts(1 hunks)src/index.css(1 hunks)src/main.tsx(2 hunks)src/vite-env.d.ts(1 hunks)tailwind.config.js(4 hunks)
💤 Files with no reviewable changes (1)
- src/app/components/fab/fab.scss
🧰 Additional context used
🪛 Biome (1.9.4)
src/app/components/modal/modalOpen.tsx
[error] 76-76: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
(lint/complexity/noUselessFragments)
src/index.css
[error] 10-10: Unknown property is not allowed.
See CSS Specifications and browser specific properties for more details.
To resolve this issue, replace the unknown property with a valid CSS property.
(lint/correctness/noUnknownProperty)
[error] 14-14: Unknown property is not allowed.
See CSS Specifications and browser specific properties for more details.
To resolve this issue, replace the unknown property with a valid CSS property.
(lint/correctness/noUnknownProperty)
[error] 24-24: Unexpected unknown function: theme
Use a known function instead.
See MDN web docs for more details.
(lint/correctness/noUnknownFunction)
[error] 25-25: Unexpected unknown function: theme
Use a known function instead.
See MDN web docs for more details.
(lint/correctness/noUnknownFunction)
[error] 26-26: Unexpected unknown function: theme
Use a known function instead.
See MDN web docs for more details.
(lint/correctness/noUnknownFunction)
[error] 28-28: Unexpected unknown function: theme
Use a known function instead.
See MDN web docs for more details.
(lint/correctness/noUnknownFunction)
[error] 29-29: Unexpected unknown function: theme
Use a known function instead.
See MDN web docs for more details.
(lint/correctness/noUnknownFunction)
[error] 43-43: Unexpected unknown function: theme
Use a known function instead.
See MDN web docs for more details.
(lint/correctness/noUnknownFunction)
[error] 44-44: Unexpected unknown function: theme
Use a known function instead.
See MDN web docs for more details.
(lint/correctness/noUnknownFunction)
[error] 48-48: Unexpected unknown function: theme
Use a known function instead.
See MDN web docs for more details.
(lint/correctness/noUnknownFunction)
[error] 49-49: Unexpected unknown function: theme
Use a known function instead.
See MDN web docs for more details.
(lint/correctness/noUnknownFunction)
[error] 50-50: Unexpected unknown function: theme
Use a known function instead.
See MDN web docs for more details.
(lint/correctness/noUnknownFunction)
[error] 52-52: Unexpected unknown function: theme
Use a known function instead.
See MDN web docs for more details.
(lint/correctness/noUnknownFunction)
[error] 76-76: Unexpected unknown function: theme
Use a known function instead.
See MDN web docs for more details.
(lint/correctness/noUnknownFunction)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Generate Lighthouse Report
- GitHub Check: Cloudflare Pages
🔇 Additional comments (69)
.pnp.cjs (8)
31-37: New Type Definitions for react-transition-group Added
A new dependency entry for@types/react-transition-group(version4.4.11) has been added here. This ensures that TypeScript type definitions are available when using thereact-transition-grouppackage, which is likely required for the new animated route map feature.
63-69: Addition of react-transition-group Dependency
The dependency entry forreact-transition-group(version4.4.5, wrapped as a virtual package) is introduced in this section. This package will provide the runtime functionality needed for transitions and animations, which are central to the new route map and ping animation features. Please verify that its version is compatible with the type definitions added above.
2729-2741: New Babel Runtime Dependency Block Introduced
A dedicated dependency block for@babel/runtime(version7.26.0) along with its companionregenerator-runtime(version0.14.1) is now present. This change should be checked against your Babel configuration to ensure consistency and that no outdated versions remain cached that might conflict with new code paths introduced by the UI enhancements.
4121-4134: Dependency Resolution Block for @types/react-transition-group
This block defines the resolution for@types/react-transition-groupby mapping it to its full package metadata, including its dependencies. It complements the entry added earlier and is essential for proper module resolution in a Yarn Plug'n'Play environment. Verify that the internalpackageDependencies(especially the link with@types/react) remain consistent with the rest of your dependency tree.
5793-5809: New dom-helpers Dependency Block Added
A new dependency block fordom-helpers(version5.2.1) is introduced, together with related dependencies likecsstypeand an association with@babel/runtime. This addition is likely aimed at supporting DOM manipulation tasks required by advanced UI animations (such as those for the route map feature). Ensure that the declaredlinkType("HARD") is appropriate and there are no conflicts with other dependencies.
7229-7235: Reaffirmation of Core Type Dependencies
This block re-lists key type packages (including@types/react-transition-groupwith version4.4.11), mirroring the earlier definitions. They appear to be part of the broader dependency mapping maintained by Yarn PnP. Confirm that this repetition is intentional (as managed by Yarn) to ensure consistency across different resolution contexts.
7261-7267: Consistent UI-Related Dependency Listing
Similar to previous segments, this block reassures the declaration of several UI-related dependencies (e.g.,react-transition-group,react-snowfall, and others). It’s important that these entries remain consistent across the file to avoid any version mismatches during runtime.
9544-9579: Dual Mapping for react-transition-group – Verify Link Types
This extensive block shows two mappings forreact-transition-group: one entry uses aSOFTlink type while the duplicate virtual entry employs aHARDlink type and includes additional package peers. This dual listing is notable; please verify that it complies with Yarn Plug'n'Play’s configuration and will not result in module resolution conflicts, ensuring the correct behavior of the new route map animations.src/app/data/routemap/circleAnimate.ts (1)
1-4: New TypeScript type for circle animationThis new type provides a structured way to represent animated elements in the route map, supporting the ping animation mentioned in the PR objectives. The type is well-defined with clear property names that indicate purpose.
src/app/data/index.ts (1)
5-5: Export CircleAnimate type properlyGood addition of the CircleAnimate type export. This resolves the previous build error that was flagged in an earlier review.
src/app/components/ptr/refreshing-content.tsx (1)
11-13: Simple formatting changeThe removal of an extra blank line is a minor formatting improvement that doesn't impact functionality.
src/vite-env.d.ts (1)
2-2: No functional changeThis appears to be just a reformatting or re-addition of the same reference type declaration, with no functional impact.
src/app/data/shuttle/organizedTimetables.ts (1)
7-7: Add documentation for the newisShuttleIpropertyThe addition of the
isShuttleIboolean property to theOrganizedTimetablestype improves type safety, but would benefit from a brief comment explaining its purpose and expected values.Consider adding a comment above this property to clarify its meaning and usage:
jungang: Array<string> + // Indicates whether this timetable is for Shuttle I route isShuttleI: boolean count: numberBased on the previous review, remember to ensure this new property is properly initialized in all places where
OrganizedTimetablesobjects are created.src/app/network/subway.ts (1)
5-5: Trailing comma style improvementThe added trailing comma is a good style improvement that makes future parameter additions cleaner in git diffs.
src/app/components/lang/i18n.ts (1)
29-29: Consistent code style improvementsThe changes to use single quotes and add trailing commas improve code style consistency throughout the file.
Also applies to: 35-38
src/main.tsx (2)
12-12: New TimeTableContextProvider implementationAdding the TimeTableContextProvider is a good approach for managing timetable state across the application, which likely supports the new route map feature mentioned in the PR objectives.
Also applies to: 50-52
44-44: Adjusted zIndex for proper visual layeringThe zIndex change from 1 to 2 ensures proper stacking order with the route map components.
Per the previous review discussion, this zIndex value places the cherry blossom effect above both the Line (z-index 0) and Circle (z-index 1) components in the route map visualization.
src/app/context/ThemeContext.tsx (1)
26-30: Improved code structure and readability.The logic for determining the theme has been reformatted into a cleaner ternary expression that makes the conditional flow more explicit while maintaining the same functionality.
src/app/network/shuttle.ts (2)
13-13: Code style improvement with trailing comma.Adding a trailing comma is a good practice that reduces git diff noise when parameters are added in the future.
16-16:✅ Verification successful
API endpoint update to support route map feature.
The endpoint has been updated from
/timetable/...to/v2/timetable/..., which likely supports the new route map functionality mentioned in the PR objectives.
🏁 Script executed:
#!/bin/bash # Verify if there are any other references to the old API endpoint that might need updating rg -i "/timetable/" --glob "*.{ts,tsx,js,jsx}" -lLength of output: 76
Verified API endpoint update in
src/app/network/shuttle.ts.
- The file now correctly uses the updated
/v2/timetable/${season}/${week}/${location}endpoint.- The only reference to
/timetable/is in this file, so no additional updates are necessary.src/app/components/fab/fab.tsx (1)
186-192: Improved readability with multi-line formatting.The
mainButtonStylesobject has been reformatted from a single line to multiple lines, making it easier to read and maintain while preserving the same functionality.src/app/components/useDarkMode.ts (3)
12-12: Code style improvement with trailing comma.Adding a trailing comma is a good practice that reduces git diff noise when parameters are added in the future.
25-25: Code style improvement with trailing comma in dependency array.Adding a trailing comma in the dependency array is consistent with modern TypeScript/React style conventions.
66-66: Code cleanup by removing trailing whitespace.Removed unnecessary trailing whitespace after the
setTheme(THEME.SPRING)call.src/app/components/routemap/Responsive.tsx (1)
1-21: Well-structured custom hook implementation.This custom hook
useResponsivefollows React best practices for tracking screen width:
- Properly initializes state with current window width
- Sets up and cleans up event listeners in useEffect
- Returns the screen width value for components to consume
This will be useful for implementing responsive behavior in the route map.
package.json (3)
11-12: Good addition of Prettier formatting script.Adding the prettier script allows for consistent code formatting across the project. This is a good practice for maintaining code quality.
35-35: Good addition of react-transition-group for animation support.Adding this dependency is appropriate for implementing the ping animation mentioned in the PR objectives. This library is well-maintained and provides a standard way to implement transitions in React.
45-45: Appropriate type definitions for react-transition-group.Adding the corresponding type definitions as a dev dependency is the correct approach when using TypeScript with external libraries.
src/app/components/subway/Subway.tsx (1)
425-425: Standardized string format for color property.Changing from single quotes to double quotes for the color property maintains consistency with string formatting elsewhere in the codebase.
src/app/components/lang/lang.en.json (2)
2-125: Inconsistent terminology between "Dorm" and "Dormitory".The file contains inconsistent terminology:
- Line 25:
"dorm": "Dorm"- Line 24:
"residence": "Dormitory"- Line 35:
"dest_dorm": "Dormitory"This inconsistency was previously flagged in a review and could confuse users. Please decide on using either "Dorm" or "Dormitory" consistently throughout the application.
13-13: Good addition of localization keys for Jungang and Yesulin.These new keys support the route map feature and ensure consistent translation across the application.
Also applies to: 27-27
src/app/components/modal/modalOpen.tsx (3)
48-48: LGTM: Code properly formatted.The removal of this empty line improves code consistency.
87-89: LGTM: Code properly formatted.Improved code formatting for the Trans component and br tag by consolidating them on single lines.
97-99: LGTM: Code properly formatted.Improved code formatting for the Trans component and br tag by consolidating them on single lines.
src/app/context/TimeTableContext.tsx (1)
1-33:✅ Verification successful
Well-implemented Context API structure.
The TimeTable context implementation follows React best practices:
- Clear interface definition
- Proper error handling in the custom hook
- Default state initialization
- Memoized context value to prevent unnecessary re-renders
Based on the past review comment, let me verify if there are any null value issues in the actual usage:
🏁 Script executed:
#!/bin/bash # Check how TimeTableContext is used in Shuttle.tsx rg -A 5 "useTimeTableContext" src/app/components/shuttle/Shuttle.tsxLength of output: 373
TimeTable Context Implementation Verified – No Null-Safety Issues Found
The implementation follows React best practices, including clear interface definition, proper error handling in the custom hook, default state initialization, and a memoized context value to prevent unnecessary re-renders. Additionally, after checking its consumption in
src/app/components/shuttle/Shuttle.tsx, the usage ofuseTimeTableContext()confirms that null values are properly handled by throwing an error if the provider is missing.src/app/components/lang/lang.ko.json (2)
12-15: Added new location aliases for route map feature.The addition of the
yesulandjungkeys provides shorter aliases for "예술인" and "중앙역" locations, which will be useful for the route map feature mentioned in the PR objectives.
25-25: Added dormitory translation key.The addition of the
dormkey for "기숙사" provides a clearer naming convention for dormitory locations in the route map.src/app/network/apiHandler.ts (1)
13-15: LGTM: Prepared type definition for future extensibility.The addition of trailing commas after the type constraint and parameter is a good practice when anticipating future additions. This makes future changes cleaner in git diffs as you won't need to modify existing lines when adding new types or parameters.
tailwind.config.js (4)
14-15: Good addition of responsive breakpoints for wider screens.These new screen sizes (
wideandmwide) will enhance the responsive design capabilities, particularly useful for the route map visualization.
35-35: Good addition of arrow color variable.The new color variable
'arrow-color'properly follows the existing pattern of using CSS variables for theming support.
47-47: Good addition of chip-orange color.Adding the orange color to the chip palette enhances the visual distinction between different types of routes or buses.
146-148: Good addition of height transition property.Adding the height transition property will enable smooth animations when expanding/collapsing elements, particularly useful for the route map container.
src/app/components/shuttle/Shuttle.tsx (8)
13-13: Good integration of context for state management.Using the TimeTableContext is a good practice for sharing timetable state across components.
339-340: Updated conditional to include R type in orange chip styling.The chip styling now properly handles both 'R' and 'NA' types with the orange color.
353-353: Good extraction of context functionality.Destructuring
setTimetablefrom the context improves code readability.
394-399: Improved condition checks for timetable data.The conditions for validating timetable data have been expanded to be more comprehensive, handling undefined and empty cases properly.
400-404: Improved state management using context.Using the context's
setTimetablefunction to update the shared state is a good practice for maintaining consistency across components.
406-406: Added setTimetable to dependency array.Including
setTimetablein the dependency array ensures the effect runs when this function reference changes.
560-561: Improved SVG fill attribute with CSS variable.Using the CSS variable for the fill attribute improves theme consistency.
576-577: Improved loader color with CSS variable.Using the CSS variable for the loader color improves theme consistency.
src/App.tsx (10)
5-5: Good addition of Transition component.Adding the Transition component from react-transition-group enables smooth animations for the route map.
10-10: Good SVG import for animation.Importing the Arrow SVG as a React component allows for proper styling and animation.
24-24: Added import for RouteMap component.Properly importing the new RouteMap component for use in the application.
127-133: Well-implemented conditional styling based on transition state.The RouteIndexCardView uses the transition state to dynamically adjust its height, creating a smooth animation effect.
139-143: Verify logic in transition states.The component is hidden on
enteredstate and fully opaque onexitedstate, which might be counter-intuitive. Ensure this matches your intended behavior.In typical transition patterns, elements are fully visible on
enteredstate and hidden/transparent onexitedstate. Confirm if this inverted logic is intentional for your specific UX requirements.
144-147: Good implementation of animated arrow rotation.The RouteToggleImage component properly rotates based on the transition state, providing a clear visual indicator of the expanded/collapsed state.
208-208: Good state management for route card visibility.Adding the routeCardClick state properly manages the visibility of the route map.
366-366: Improved refreshing component with theme awareness.Passing the theme to the Refreshing component ensures consistent styling.
384-385: Improved SVG fill attribute with CSS variable.Using the CSS variable for the fill attribute improves theme consistency.
456-491: Well-implemented transition for route map visibility.The Transition component properly manages the animation states for the RouteIndexCardView, providing a smooth user experience.
src/app/components/routemap/RouteMap.tsx (4)
1-7: Well-structured imports and dependencies.The component imports all necessary dependencies for styling, internationalization, and custom components.
8-22: Good responsive styling implementation.The styled components use twin.macro effectively to create responsive layouts with proper grid structures and text sizing based on screen width and language.
36-78: Well-structured route map component.The component is well-organized with clear separation of concerns between layout, styling, and content. Using the useTranslation hook for internationalization is a good practice.
60-74: Good implementation of route visualizations.Each route method is properly paired with its corresponding RouteVisual component, passing the necessary props for proper rendering.
src/app/components/fulltime/FullTime.tsx (3)
147-148: No concerns here—change looks fine.
Applying the fill color variable is consistent with your theming approach.
175-178: NewTimeBoxBodyGridforyesullooks good.
Grouping theprops.directYminutes under a green chip is consistent with your color-coded approach.
309-311: Conditional rendering to skip empty schedules is effective.
Avoiding the creation of an empty container ensures a cleaner UI and reduces DOM clutter.src/index.css (1)
6-15:❓ Verification inconclusive
Remove portable-unfriendly properties or verify browser support.
Properties like-khtml-user-selectand-user-dragare not widely recognized and trigger warnings in static analysis. Confirm ongoing necessity or remove them for cleaner cross-browser compatibility.
🏁 Script executed:
#!/bin/bash # Attempt to identify usage or references to these properties beyond index.css rg '-khtml-user-select|-user-drag'Length of output: 441
Below is a revised review comment. Before finalizing changes, please manually verify that these legacy properties aren’t used elsewhere in the repository (especially since our initial regex search produced an error). You might want to re-run a search using a properly escaped regex to ensure no unintentional references exist.
Action Required: Verify Necessity of Legacy CSS Properties
The CSS in
src/index.cssincludes legacy vendor-specific properties (-khtml-user-selectand-user-drag) that are not widely supported and can trigger static analysis warnings. Our initial search encountered errors due to regex interpretation issues, so please verify manually that these properties are confined only to this file before removing or justifying their inclusion.
- Context: Legacy properties like
-khtml-user-selectand-user-dragare used only in this file (as far as can be judged by our search attempt).- Recommendation: If supporting outmoded browsers is not a requirement, please remove these properties. Alternatively, document why they are still needed if browser support is indeed required.
Diff snippet under review:
body { -webkit-touch-callout: none; -webkit-user-select: none; -webkit-user-drag: none; -khtml-user-select: none; -moz-user-select: none; -ms-user-select: none; user-select: none; -user-drag: none; }For further verification, you can run the following shell script with a properly escaped regex:
Please confirm that no additional references exist before proceeding with removal or adding further documentation.
🏁 Script executed:
#!/bin/bash # Re-run search command using proper escaping to locate any occurrence of the problematic properties. rg -e '\-khtml-user-select|\-user-drag'Length of output: 457
Legacy Vendor-Specific CSS Properties Usage
The legacy vendor properties (
-khtml-user-selectand-user-drag) appear in bothsrc/index.css(lines 6–15) andsrc/app/components/fab/fab.scss. These properties are not widely supported and can trigger static analysis warnings.
- Action: Verify whether these properties are truly required for supporting specific browsers.
- Recommendation: If legacy support isn’t necessary, remove them from both files. Otherwise, add documentation clarifying why they must remain.
Here’s the current snippet from
src/index.css:body { -webkit-touch-callout: none; -webkit-user-select: none; -webkit-user-drag: none; -khtml-user-select: none; -moz-user-select: none; -ms-user-select: none; user-select: none; -user-drag: none; }Please ensure that similar properties in
src/app/components/fab/fab.scssare also reviewed.🧰 Tools
🪛 Biome (1.9.4)
[error] 10-10: Unknown property is not allowed.
See CSS Specifications and browser specific properties for more details.
To resolve this issue, replace the unknown property with a valid CSS property.(lint/correctness/noUnknownProperty)
[error] 14-14: Unknown property is not allowed.
See CSS Specifications and browser specific properties for more details.
To resolve this issue, replace the unknown property with a valid CSS property.(lint/correctness/noUnknownProperty)
src/app/components/routemap/Animation.tsx (1)
8-11: Consider using Tailwind's group-based visibility controlThe PingCircle component uses a prop to control visibility through the
hiddenclass. For animations, consider using Tailwind's group functionality for more declarative control over visibility and animation states.-const PingCircle = styled.div<{ $on: boolean }>` - ${tw`animate-ping absolute rounded-full inline-flex h-3 w-3 rt1:h-2.5 rt1:w-2.5 z-[0] mx-2`} - ${(props) => (props.$on ? tw`` : tw`hidden`)} +const PingCircle = styled.div<{ $on: boolean }>` + ${tw`animate-ping absolute rounded-full inline-flex h-3 w-3 rt1:h-2.5 rt1:w-2.5 z-[0] mx-2`} + ${(props) => (props.$on ? tw`` : tw`!opacity-0 !scale-0`)}This approach maintains the component in the DOM but visually hides it, which can provide smoother transitions when the element becomes visible again.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/app/components/routemap/Animation.tsx (3)
140-165: Improve Animation component with TypeScript enums and proper defaultsThe current switch statement relies on string literals for route types. Using TypeScript enums would make this more maintainable and type-safe.
+ enum RouteStatus { + Direct = 'direct', + Cycle = 'cycle', + Yesulin = 'yesulin', + Jungang = 'jungang' + } + + interface AnimationProps { + isOn: boolean; + index: number; + rootStatus: RouteStatus | string; + } - const Animation = (props: { - isOn: boolean - index: number - rootStatus: string - }) => { + const Animation = ({ isOn, index, rootStatus }: AnimationProps) => { switch (rootStatus) { - case 'direct': + case RouteStatus.Direct: - if (props.index === 4) - return <PingCircle className="bg-chip-orange" $on={props.isOn} /> - return <PingCircle className="bg-chip-blue" $on={props.isOn} /> + if (index === 4) + return <PingCircle className="bg-chip-orange" $on={isOn} /> + return <PingCircle className="bg-chip-blue" $on={isOn} /> // Similar changes for other cases... default: - return <></> + return null } }
42-47: Add TypeScript types to timetableType functionThe
timetableTypefunction should have proper TypeScript type definitions for parameters and return values, improving type safety.- const timetableType = (type: string, index: number) => { + const timetableType = (type: string, index: number): CircleAnimate => { if (type === 'C') return { dotType: 'cycle', index: index } else if (type === 'DHJ') return { dotType: 'jungang', index: index } else if (type === 'DY') return { dotType: 'yesulin', index: index } else return { dotType: 'direct', index: index } }
8-11: Consider accessibility improvements for PingCircle componentThe PingCircle component creates an animated visual indicator but lacks accessibility attributes for users with disabilities.
- const PingCircle = styled.div<{ $on: boolean }>` + const PingCircle = styled.div<{ $on: boolean; $ariaLabel?: string }>` ${tw`animate-ping absolute rounded-full inline-flex h-3 w-3 rt1:h-2.5 rt1:w-2.5 z-[0] mx-2`} ${(props) => (props.$on ? tw`visible` : tw`invisible`)} ` // Then in the Animation component: + return <PingCircle + className="bg-chip-blue" + $on={isOn} + $ariaLabel="Active blue route point" + aria-hidden={!isOn} + role="status" + />This improvement would help screen readers understand the purpose of these animated elements.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/app/components/routemap/Animation.tsx(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Cloudflare Pages
🔇 Additional comments (2)
src/app/components/routemap/Animation.tsx (2)
49-94: Standardize error handling across all route conditionsThe function only checks for invalid timetable.type ('NA') in the 'shuttlecoke_i' case, but not in other cases. This inconsistency could lead to bugs.
export const CircleAnimateType = ( tab: string, timetable: SingleShuttleSchedule, ) => { if (timetable?.time !== '') { + // If timetable type is NA, no animations are needed + if (timetable.type === 'NA') return [] + switch (tab) { case 'shuttlecoke_o': return [timetableType(timetable.type, 1)] case 'subway': return [timetableType(timetable.type, 2)] case 'yesulin': return [timetableType(timetable.type, 3)] case 'jungang': return [ { dotType: 'jungang', index: 3, }, ] case 'shuttlecoke_i': - if (timetable.type === 'NA') return [] return [ // ... existing items ]
96-138: Optimize useAnimation hook to prevent unnecessary rendersThe hook recreates the
tableArrayon every render before checking if the timetable has changed. This is inefficient and could be optimized.export const useAnimation = (tab: string) => { const [timetable, setTimetable] = React.useState<{ direct: Array<boolean> cycle: Array<boolean> yesulin: Array<boolean> jungang: Array<boolean> }>({ direct: [false, false, false, false, false], cycle: [false, false, false, false, false, false], yesulin: [false, false, false, false, false], jungang: [false, false, false, false, false, false], }) const timetables = useTimeTableContext().timetable const checkTable = React.useRef<SingleShuttleSchedule>() - React.useEffect(() => { - checkTable.current = undefined - }, []) useEffect(() => { + // Early return if timetable hasn't changed + if (checkTable.current === timetables) return + + checkTable.current = timetables + let tableArray = { direct: [false, false, false, false, false], cycle: [false, false, false, false, false, false], yesulin: [false, false, false, false, false], jungang: [false, false, false, false, false, false], } - setTimetable(tableArray) - if (checkTable.current === timetables) return - - checkTable.current = timetables const animateType = CircleAnimateType(tab, timetables) animateType.forEach((item) => { tableArray = circleAnimation(item, tableArray) }) setTimetable(tableArray) }, [timetables, tab]) return timetable }
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (6)
src/app/components/shuttle/Shuttle.tsx (1)
382-405: Consider merging the useEffect hooks.There's potential for merging the two useEffect hooks that manage timetable state and re-rendering logic, as they have overlapping dependencies and functionality.
// Recalculate & Rerender every second useEffect(() => { const timer = setTimeout(() => { setCurrentTime(new Date().getTime()) }, 1000) + const filtered = timetable.data?.filter((val) => isAfterCurrentTime(val)) + if ( + timetable.data?.length === 0 || + timetable.status !== 'success' || + filtered === undefined || + filtered.length === 0 + ) { + setTimetable({ time: '', type: 'NA' }) + setTimetableAlive(false) + } else { + setTimetable(filtered[0]) + setTimetableAlive(true) + } return () => clearTimeout(timer) - }, [timetable.data, currentTime]) + }, [timetable.data, timetable.status, setTimetable, currentTime]) - // For info card to not show when error or no shuttle available - useEffect(() => { - const filtered = timetable.data?.filter((val) => isAfterCurrentTime(val)) - if ( - timetable.data?.length === 0 || - timetable.status !== 'success' || - filtered === undefined || - filtered.length === 0 - ) { - setTimetable({ time: '', type: 'NA' }) - setTimetableAlive(false) - } else { - setTimetable(filtered[0]) - setTimetableAlive(true) - } - }, [timetable.data, timetable.status, setTimetable, currentTime])src/app/data/routemap/dotAnimate.ts (1)
1-4: Consider using a union type for more robust type safety.While defining a
dotTypestring is flexible, restricting valid values with a union type (e.g.,'direct' | 'cycle' | 'yesulin' | 'jungang') can catch errors at compile time.-export type DotAnimationConfig = { - dotType: string - index: number -} +export type DotAnimationConfig = { + dotType: 'direct' | 'cycle' | 'yesulin' | 'jungang' + index: number +}src/app/components/routemap/RouteVisual.tsx (2)
60-78: Consider refactoring or documenting this extensive switch logic.
isPrevStopbehaves differently per case, and some conditions (e.g., foryesulinandjungang) might be tricky to maintain over time. A modular or data-driven approach (e.g., mapping route types to conditions) can enhance clarity.
160-176: Reduce nested conditional rendering for improved readability.The snippet that conditionally maps and groups route elements (lines 160-176) is complex. Extracting the logic that handles
index >= 2 && index <= 4into a helper function or separate component would make the code more understandable and maintainable.src/app/components/routemap/DotAnimation.tsx (2)
17-36: Document or guard against potential index shifts inapplyDotAnimationFlag.Logic like
newFlagArray.direct[i >= 3 ? i + 1 : i] = truecould lead to out-of-bounds array access for higher indexes, and the reason for shifting indices may not be obvious. Adding comments or boundary checks can prevent hidden errors.
46-91: Large switch statement—consider consolidating repetitive branches.
getDotAnimationConfigsplits logic in multiple cases, some returning similar structures. A table-driven approach or parameterized logic can reduce repetition and streamline maintenance.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/app/components/routemap/DotAnimation.tsx(1 hunks)src/app/components/routemap/RouteVisual.tsx(1 hunks)src/app/components/shuttle/Shuttle.tsx(6 hunks)src/app/data/index.ts(1 hunks)src/app/data/routemap/dotAnimate.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/app/data/index.ts
🧰 Additional context used
🧬 Code Definitions (3)
src/app/data/routemap/dotAnimate.ts (1)
src/app/data/index.ts (2) (2)
DotAnimationConfig(6-6)RouteAnimationFlag(7-7)
src/app/components/routemap/DotAnimation.tsx (1)
src/app/data/routemap/dotAnimate.ts (2) (2)
DotAnimationConfig(1-4)RouteAnimationFlag(6-11)
src/app/components/shuttle/Shuttle.tsx (1)
src/app/context/TimeTableContext.tsx (1) (1)
useTimeTableContext(12-16)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Cloudflare Pages
🔇 Additional comments (12)
src/app/components/shuttle/Shuttle.tsx (8)
13-13: Good addition of the TimeTableContext.The introduction of the
useTimeTableContexthook aligns with the PR's objective of improving route map functionality by centralizing timetable state management, making it accessible across components.
353-353: Clean implementation of context extraction.The destructured assignment of
setTimetablefrom the context hook follows React best practices and prepares the component to update the shared timetable state.
396-398: Improved error handling for filtered bus data.Your enhanced conditional check properly handles both undefined data and empty arrays, fixing the issue mentioned in previous reviews about empty filtered bus data.
399-404: Good timetable state management with context.The implementation now correctly updates the shared timetable context based on data availability, using appropriate default values when no data is available.
405-405: Correct dependency array update.Adding
setTimetableandcurrentTimeto the dependency array ensures the effect runs when these values change, maintaining data consistency throughout the application.
339-340: Enhanced chip type handling.The condition now correctly handles both 'R' and 'NA' types with the same styling, which improves the visual consistency of the route map display.
559-559: Consistent styling with CSS variables.Using
var(--color-theme-text)instead of hardcoded values improves theme support and aligns with the rest of the codebase's styling approach.
575-575: Consistent styling with CSS variables.Using
var(--color-load-color)instead of hardcoded color values maintains consistency with the theming system and improves dark/light mode support.src/app/data/routemap/dotAnimate.ts (1)
6-11: Looks good—explicit properties promote clarity.Defining these four route arrays as separate properties fosters clear usage and consistent structure.
src/app/components/routemap/RouteVisual.tsx (1)
80-101:✅ Verification successful
Verify array index boundaries for
AddLines.The expression
lineIndex[props.rootStatus][props.index]may raise an index-out-of-range error ifprops.indexexceeds the length oflineIndex[props.rootStatus]. Consider validating or gracefully handling out-of-range conditions.
🏁 Script executed:
#!/bin/bash # Search for all calls to AddLines to see how props.index is used or derived. rg -A 5 "AddLines" .Length of output: 871
Array Index Safety Validated for AddLines
Our investigation confirms that the only call to
AddLinesderivesprops.indexvialineIndex[props.rootStatus].indexOf(item), which returns-1when the item isn’t found—and the early return onprops.index === -1effectively guards against an out-of-range access. While the current usage is safe, if this component is ever used with indices from alternative sources, it would be wise to add a check ensuring thatprops.indexis less thanlineIndex[props.rootStatus].length.src/app/components/routemap/DotAnimation.tsx (2)
93-129: Good structure for the custom hook.
useDotAnimationclearly manages state re-initialization and applies flags from the timetable. This is straightforward and maintainable.
131-140: Return early pattern is effective.Avoiding rendering at
yesulinroute index 2 is a clean approach. Ensure that skipping the render is indeed the intended UX.
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a dynamic route map for shuttle bus navigation with animated route indicators and enhanced timetable context management. Key changes include the addition of a TimeTable context provider, new RouteMap and DotAnimation components to drive interactive animations, and various UI and error fixes across shuttle, subway, fulltime, and modal components.
Reviewed Changes
Copilot reviewed 29 out of 34 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/app/context/TimeTableContext.tsx | New context provider to manage shuttle timetable state. |
| src/app/components/routemap/RouteMap.tsx | Creation of the RouteMap component using styled components and i18n for dynamic route labels. |
| src/app/components/routemap/DotAnimation.tsx | New dot animation logic with functions to update animation flags based on timetable data. |
| src/app/components/routemap/RouteVisual.tsx | Integration of DotAnimation and additional UI elements to visually represent route status. |
| src/App.tsx | UI transition updates including a new RouteMap integration and refined styling for route cards. |
| src/app/components/subway/Subway.tsx | Fixes to error values in window.open target and attribute formatting. |
| src/app/components/fulltime/FullTime.tsx | Adjustment of dynamic height calculations and chip display logic in timetable view. |
| src/app/components/shuttle/Shuttle.tsx | Updates to chip color logic and timetable state management, ensuring proper timetable display. |
| src/app/components/modal/modalOpen.tsx | Minor formatting adjustments in changelog rendering and translation key usage. |
| src/app/context/ThemeContext.tsx | Refinement of theme state initialization from localStorage. |
| src/app/data/index.ts | Exports updated to include new dot animation types. |
| src/app/components/lang/i18n.ts | Consistent formatting for i18n configuration. |
| src/app/components/useDarkMode.ts | Minor code style adjustments and removal of unneeded empty lines. |
| src/app/components/fab/fab.tsx | UI style improvements with adjusted inline styling for the floating action button. |
Files not reviewed (5)
- .pnp.cjs: Language not supported
- package.json: Language not supported
- src/app/components/fab/fab.scss: Language not supported
- src/app/components/lang/lang.en.json: Language not supported
- src/app/components/lang/lang.ko.json: Language not supported
Comments suppressed due to low confidence (2)
src/app/components/shuttle/Shuttle.tsx:396
- The check for 'filtered === undefined' is unnecessary because Array.filter always returns an array. Removing this redundant condition would simplify the logic.
if (filtered === undefined || filtered.length === 0) {
src/app/components/shuttle/Shuttle.tsx:339
- The newly added condition for chipType 'R' or 'NA' assigns an orange background, which differs from the previous default styling. Please verify that this change is intentional and aligns with the design specifications.
else if (chipType == 'R' || chipType == 'NA') {
- Refactor DotAnimation - Fix Shuttle to sync with timetable - Fix FullTime and Shuttle to account for simultaneous shuttle bus arrivals
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/app/components/routemap/DotAnimation.tsx (1)
39-47:⚠️ Potential issuePotential out-of-bounds array access in applyDotAnimationFlag.
The current implementation could potentially access array indices beyond their bounds, especially for the 'direct' case where
i >= 3leads toi + 1.Add bounds checking to prevent potential runtime errors:
- for (let i = config.index; i <= config.index + 1; i++) { - if (config.dotType === 'direct') - newFlagArray.direct[i >= 3 ? i + 1 : i] = true - if (config.dotType === 'cycle') newFlagArray.cycle[i] = true - if (config.dotType === 'yesulin') - newFlagArray.yesulin[i === 2 ? 3 : i] = true - if (config.dotType === 'jungang') newFlagArray.jungang[i] = true - } + for (let i = config.index; i <= config.index + 1; i++) { + if (config.dotType === 'direct') { + const index = i >= 3 ? i + 1 : i; + if (index < newFlagArray.direct.length) { + newFlagArray.direct[index] = true; + } + } + if (config.dotType === 'cycle' && i < newFlagArray.cycle.length) { + newFlagArray.cycle[i] = true; + } + if (config.dotType === 'yesulin') { + const index = i === 2 ? 3 : i; + if (index < newFlagArray.yesulin.length) { + newFlagArray.yesulin[index] = true; + } + } + if (config.dotType === 'jungang' && i < newFlagArray.jungang.length) { + newFlagArray.jungang[i] = true; + } + }
🧹 Nitpick comments (10)
src/app/context/TimeTableContext.tsx (2)
12-12: Consider providing a non-null default value for better type safety.Currently, the context is initialized with
nullwhich requires explicit null checking in theuseTimeTableContexthook. Consider providing a default implementation instead:- const TimeTableContext = React.createContext<TimeTableContextProps | null>(null) + const defaultContext: TimeTableContextProps = { + currTimetable: [], + setCurrTimetable: () => { + throw new Error('TimeTable provider not implemented'); + }, + }; + const TimeTableContext = React.createContext<TimeTableContextProps>(defaultContext)This approach would eliminate the need for null checks and provide better type safety.
20-37: Add documentation for the TimeTableContextProvider component.Consider adding JSDoc comments to describe the purpose and usage of this context provider. This will help other developers understand how to use it correctly.
/** * Provider component for the TimeTable context. * Manages the current timetable state and provides it to all child components. */ export const TimeTableContextProvider = ({ children, }: React.PropsWithChildren) => {src/app/components/routemap/RouteVisual.tsx (4)
60-78: Consider simplifying the complex isPrevStop function.The isPrevStop function contains nested conditions and multiple special cases, making it difficult to understand and maintain. This complexity increases the risk of bugs when modifying the code.
You could refactor this to use a lookup table approach:
- const isPrevStop = (line: string, index: number, tab: string) => { - switch (tab) { - case 'shuttlecoke_o': - return index !== 0 - case 'shuttlecoke_i': - return index >= 4 - case 'subway': - if (line === 'yesulin') return false - return index >= 2 - case 'yesulin': - if (line === 'direct' || line === 'jungang') return false - return index >= 3 - case 'jungang': - if (line === 'jungang') return index >= 3 - return false - default: - return true - } - } + // Define a mapping of conditions + const prevStopConditions = { + shuttlecoke_o: { + default: (index: number) => index !== 0 + }, + shuttlecoke_i: { + default: (index: number) => index >= 4 + }, + subway: { + yesulin: () => false, + default: (index: number) => index >= 2 + }, + yesulin: { + direct: () => false, + jungang: () => false, + default: (index: number) => index >= 3 + }, + jungang: { + jungang: (index: number) => index >= 3, + default: () => false + } + } + + const isPrevStop = (line: string, index: number, tab: string) => { + // Get conditions for the tab, or default to returning true + const tabConditions = prevStopConditions[tab] || { default: () => true }; + + // Use line-specific condition if available, otherwise use default for the tab + return (tabConditions[line] || tabConditions.default)(index); + }
87-100: The color logic for lines could be simplified.The nested ternary operators make the code harder to read and understand.
Consider refactoring to make the logic clearer:
- className={ - isPrevStop(props.rootStatus, index, props.tab) - ? index === 4 - ? 'bg-chip-orange' - : routeColorTable[props.rootStatus] - : 'bg-zinc-200 dark:bg-slate-500' - } + className={ + !isPrevStop(props.rootStatus, index, props.tab) + ? 'bg-zinc-200 dark:bg-slate-500' + : index === 4 + ? 'bg-chip-orange' + : routeColorTable[props.rootStatus] + }Or even better, extract this logic to a separate function:
+ const getLineColor = (rootStatus: string, index: number, tab: string) => { + if (!isPrevStop(rootStatus, index, tab)) { + return 'bg-zinc-200 dark:bg-slate-500'; + } + return index === 4 ? 'bg-chip-orange' : routeColorTable[rootStatus]; + } // Then in the component: - className={ - isPrevStop(props.rootStatus, index, props.tab) - ? index === 4 - ? 'bg-chip-orange' - : routeColorTable[props.rootStatus] - : 'bg-zinc-200 dark:bg-slate-500' - } + className={getLineColor(props.rootStatus, index, props.tab)}
112-156: Complex route rendering logic could be simplified.The
AddElementsfunction contains complex logic with multiple nested conditions and special casing, making it difficult to understand and modify.Consider breaking down the large function into smaller, more focused helper functions:
- Create a separate function for dot class generation
- Create a separate function for special stops text
- Extract the complex logic for arrChild into its own function
For example:
+ const getDotClass = (rootStatus: string, item: number, tab: string) => { + const baseClass = isPrevStop(rootStatus, item, tab) + ? item >= 5 + ? 'bg-chip-orange' + : routeColorTable[rootStatus] + : 'bg-zinc-200 dark:bg-slate-500'; + + const opacity = item === 2 && rootStatus === 'yesulin' ? ' opacity-0' : ''; + return baseClass + opacity; + } + + const renderSpecialStopsText = (rootStatus: string, item: number, tab: string, lang: string) => { + if (item !== 3) return <></>; + + const textClass = isPrevStop(rootStatus, item, tab) + ? routeColorTable[rootStatus + 'Text'] + : 'text-zinc-200 dark:text-slate-500'; + + return ( + <SpecialStopsText key={0} lang={lang} className={textClass}> + {rootStatus === 'jungang' ? t('jung') : t('yesul')} + </SpecialStopsText> + ); + }
158-178: Simplify the complex layout logic.The map function with conditional returns and array manipulation is hard to understand and maintain.
Consider refactoring this to separate the special case handling for direct vs. non-direct routes:
- return ( - <> - {arrDots.map((element: React.JSX.Element, index) => { - if (index >= 2 && index <= 4 && props.rootStatus !== 'direct') { - arrChild.push(element) - if (index === 4) { - return ( - <div - key={index} - className="col-span-2 grid grid-cols-3 w-[75%] place-items-center" - > - {arrChild.map((element) => { - return element - })} - </div> - ) - } - } else return element - })} - </> - ) + // For direct route, just return all elements + if (props.rootStatus === 'direct') { + return <>{arrDots}</>; + } + + // For non-direct routes, handle special layout for middle elements + const beforeMiddle = arrDots.slice(0, 2); + const middle = arrDots.slice(2, 5); + const afterMiddle = arrDots.slice(5); + + return ( + <> + {beforeMiddle} + <div className="col-span-2 grid grid-cols-3 w-[75%] place-items-center"> + {middle} + </div> + {afterMiddle} + </> + );src/app/components/routemap/DotAnimation.tsx (4)
143-149: Use forEach instead of map when not returning a value.When not using the return value of map, it's better to use forEach for clarity.
- currTimetableArray.map((currTimetable: SingleShuttleSchedule) => { + currTimetableArray.forEach((currTimetable: SingleShuttleSchedule) => {
163-165: Missing return in conditional branch.When
props.rootStatus === 'yesulin' && props.index === 2, the function doesn't have an explicit return statement. While JavaScript will implicitly return undefined, it's better to be explicit for clarity.- if (props.rootStatus === 'yesulin' && props.index === 2) return + if (props.rootStatus === 'yesulin' && props.index === 2) return null;
131-138: Consider extracting the flag table initialization.The flag table initialization appears in two places (lines 117-122 and 131-136). Consider extracting it to a separate function or constant to avoid duplication.
+ const getInitialFlagTable = () => ({ + direct: [false, false, false, false, false, false], + cycle: [false, false, false, false, false, false], + yesulin: [false, false, false, false, false, false], + jungang: [false, false, false, false, false, false], + }); const [flagTable, setFlagTable] = React.useState<RouteAnimationFlag>( - { - direct: [false, false, false, false, false, false], - cycle: [false, false, false, false, false, false], - yesulin: [false, false, false, false, false, false], - jungang: [false, false, false, false, false, false], - } + getInitialFlagTable() ) useEffect(() => { - let tempFlagTable = { - direct: [false, false, false, false, false, false], - cycle: [false, false, false, false, false, false], - yesulin: [false, false, false, false, false, false], - jungang: [false, false, false, false, false, false], - } + let tempFlagTable = getInitialFlagTable();
74-111: Simplify switch statement in getDotAnimationConfig.The switch statement is quite large with special case handling. Consider simplifying by extracting the logic for special cases.
+ const getShuttlecokeIConfig = (currTimetable: SingleShuttleSchedule) => { + if (currTimetable.type === 'NA') return []; + + return [ + { dotType: 'direct', index: 3 }, + { dotType: 'jungang', index: 4 }, + { dotType: 'cycle', index: 4 }, + { dotType: 'yesulin', index: 4 } + ]; + } switch (tab) { case 'shuttlecoke_o': return [getDotAniTimetableConfig(currTimetable.type, 1)] case 'subway': return [getDotAniTimetableConfig(currTimetable.type, 2)] case 'yesulin': return [getDotAniTimetableConfig(currTimetable.type, 3)] case 'jungang': return [ { dotType: 'jungang', index: 3, }, ] case 'shuttlecoke_i': - if (currTimetable.type === 'NA') return [] - - return [ - { - dotType: 'direct', - index: 3, - }, - { - dotType: 'jungang', - index: 4, - }, - { - dotType: 'cycle', - index: 4, - }, - { - dotType: 'yesulin', - index: 4, - }, - ] + return getShuttlecokeIConfig(currTimetable); default: return [getDotAniTimetableConfig(currTimetable.type, 0)] }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/app/components/fulltime/FullTime.tsx(8 hunks)src/app/components/routemap/DotAnimation.tsx(1 hunks)src/app/components/routemap/RouteVisual.tsx(1 hunks)src/app/components/shuttle/Shuttle.tsx(6 hunks)src/app/context/TimeTableContext.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/app/components/shuttle/Shuttle.tsx
- src/app/components/fulltime/FullTime.tsx
🧰 Additional context used
🧬 Code Definitions (1)
src/app/components/routemap/RouteVisual.tsx (2)
src/app/data/routemap/dotAnimate.ts (1) (1)
RouteAnimationFlag(6-11)src/app/components/routemap/DotAnimation.tsx (1) (1)
useDotAnimation(116-155)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Cloudflare Pages
🔇 Additional comments (3)
src/app/context/TimeTableContext.tsx (3)
14-18: LGTM! Good error handling in the custom hook.The
useTimeTableContexthook appropriately throws an error when used outside of its provider, making it easier to debug context-related issues.
23-25: Initialize with empty array addresses the previous null check concern.Initializing
currTimetablewith an empty array instead of null is a good practice. This addresses the concern raised in previous reviews about potential null reference errors.
27-30: Good use of useMemo for performance optimization.The
useMemohook is correctly implemented to only recalculate the context value whencurrTimetablechanges. SincesetCurrTimetableis stable between renders (provided by useState), it doesn't need to be included in the dependency array.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
src/app/components/routemap/DotAnimation.tsx (1)
29-48:⚠️ Potential issueFix potential array bounds issue in applyDotAnimationFlag.
There's a potential out-of-bounds array access in the
directcase wheni >= 3. If thedirectarray length is not sufficient, it could lead to runtime errors.Add bounds checking to prevent out-of-bounds access:
export const applyDotAnimationFlag = ( config: DotAnimationConfig, flagArray: RouteAnimationFlag, ) => { const newFlagArray = { direct: [...flagArray.direct], cycle: [...flagArray.cycle], yesulin: [...flagArray.yesulin], jungang: [...flagArray.jungang], } for (let i = config.index; i <= config.index + 1; i++) { - if (config.dotType === 'direct') - newFlagArray.direct[i >= 3 ? i + 1 : i] = true - if (config.dotType === 'cycle') newFlagArray.cycle[i] = true - if (config.dotType === 'yesulin') - newFlagArray.yesulin[i === 2 ? 3 : i] = true - if (config.dotType === 'jungang') newFlagArray.jungang[i] = true + if (config.dotType === 'direct') { + const index = i >= 3 ? i + 1 : i; + if (index < newFlagArray.direct.length) { + newFlagArray.direct[index] = true; + } + } + if (config.dotType === 'cycle' && i < newFlagArray.cycle.length) { + newFlagArray.cycle[i] = true; + } + if (config.dotType === 'yesulin') { + const index = i === 2 ? 3 : i; + if (index < newFlagArray.yesulin.length) { + newFlagArray.yesulin[index] = true; + } + } + if (config.dotType === 'jungang' && i < newFlagArray.jungang.length) { + newFlagArray.jungang[i] = true; + } } return newFlagArray }src/app/components/shuttle/Shuttle.tsx (1)
486-492:⚠️ Potential issueAdd error handling for empty filtered array.
The current implementation might cause errors if
filteredis empty orcurrTimetableis undefined when accessing their first elements.// Send filtered[0](or also include filtered[1] when bus arrive simultaneously) Array to RouteVisual // when filtered has been updated. - if (filtered[0] !== currTimetable[0]) { + if (filtered.length > 0 && (currTimetable.length === 0 || filtered[0] !== currTimetable[0])) { if (filtered.length >= 2 && filtered[0].time === filtered[1].time) setCurrTimetable([filtered[0], filtered[1]]) else setCurrTimetable([filtered[0]]) }
🧹 Nitpick comments (7)
src/app/components/routemap/RouteVisual.tsx (3)
64-82: Complex logic in isPrevStop function.The
isPrevStopfunction has complex branching logic with multiple switch cases and nested conditions. This approach can make the code harder to maintain and reason about.Consider refactoring this to use a more declarative approach with lookup tables or a more structured data model. For example:
-const isPrevStop = (line: string, index: number, tab: string) => { - switch (tab) { - case 'shuttlecoke_o': - return index !== 0 - case 'shuttlecoke_i': - return index >= 4 - case 'subway': - if (line === 'yesulin') return false - return index >= 2 - case 'yesulin': - if (line === 'direct' || line === 'jungang') return false - return index >= 3 - case 'jungang': - if (line === 'jungang') return index >= 3 - return false - default: - return true - } -} +const prevStopRules = { + shuttlecoke_o: (line: string, index: number) => index !== 0, + shuttlecoke_i: (line: string, index: number) => index >= 4, + subway: (line: string, index: number) => line !== 'yesulin' && index >= 2, + yesulin: (line: string, index: number) => !(line === 'direct' || line === 'jungang') && index >= 3, + jungang: (line: string, index: number) => line === 'jungang' && index >= 3, + default: () => true +}; + +const isPrevStop = (line: string, index: number, tab: string) => { + const rule = prevStopRules[tab as keyof typeof prevStopRules] || prevStopRules.default; + return rule(line, index); +}
107-162: Complex className composition in RouteElement.The className composition in the Dot component is complex with multiple nested ternary operators, making the code harder to read and maintain.
Consider extracting this logic into a separate function to improve readability:
+const getDotClassName = (routeStatus: string, item: number, tab: string) => { + const baseClass = isPrevStop(routeStatus, item, tab) + ? item >= 5 + ? 'bg-chip-orange' + : routeColorTable[routeStatus] + : 'bg-zinc-200 dark:bg-slate-500'; + + return routeStatus === 'yesulin' && item === 2 + ? `${baseClass} opacity-0` + : baseClass; +} const RouteElement = (props: { routeStatus: string tab: string animationFlagTable: { [key: string]: Array<boolean> } item: number }) => { const { t, i18n } = useTranslation() return ( <RouteStations> <Dot - className={ - (isPrevStop(props.routeStatus, props.item, props.tab) - ? props.item >= 5 - ? 'bg-chip-orange' - : routeColorTable[props.routeStatus] - : 'bg-zinc-200 dark:bg-slate-500') + - (props.item === 2 && props.routeStatus === 'yesulin' - ? ' opacity-0' - : '') - } + className={getDotClassName(props.routeStatus, props.item, props.tab)} >
164-216: Complex rendering logic in RouteElementGroup.The
RouteElementGroupcomponent contains complex filtering and conditional rendering logic that makes the code harder to understand and maintain.Consider refactoring this component to use a more data-driven approach with clear separation of logic and presentation:
const RouteElementGroup = (props: { routeStatus: string tab: string animationFlagTable: { [key: string]: Array<boolean> } }) => { + // Extract the filtering logic + const getFilteredIndices = () => { + return dotIndex[props.routeStatus].filter( + (item) => !(props.routeStatus !== 'direct' && item > 2 && item <= 4) + ); + } + + // Separate the rendering of special case + const renderSpecialCase = (item: number) => ( + <div + key={item} + className={'col-span-2 grid grid-cols-3 w-[75%] place-items-center'} + > + <RouteElement + routeStatus={props.routeStatus} + tab={props.tab} + animationFlagTable={props.animationFlagTable} + item={item} + /> + <RouteElement + routeStatus={props.routeStatus} + tab={props.tab} + animationFlagTable={props.animationFlagTable} + item={item + 1} + /> + <RouteElement + routeStatus={props.routeStatus} + tab={props.tab} + animationFlagTable={props.animationFlagTable} + item={item + 2} + /> + </div> + ); + + // Separate the rendering of standard case + const renderStandardCase = (item: number) => ( + <RouteElement + key={item} + routeStatus={props.routeStatus} + tab={props.tab} + animationFlagTable={props.animationFlagTable} + item={item} + /> + ); return ( <> - {dotIndex[props.routeStatus] - .filter( - (item) => !(props.routeStatus !== 'direct' && item > 2 && item <= 4), - ) - .map((item) => { - if (item === 2 && props.routeStatus !== 'direct') - return ( - <div - key={item} - className={ - 'col-span-2 grid grid-cols-3 w-[75%] place-items-center' - } - > - <RouteElement - routeStatus={props.routeStatus} - tab={props.tab} - animationFlagTable={props.animationFlagTable} - item={item} - /> - <RouteElement - routeStatus={props.routeStatus} - tab={props.tab} - animationFlagTable={props.animationFlagTable} - item={item + 1} - /> - <RouteElement - routeStatus={props.routeStatus} - tab={props.tab} - animationFlagTable={props.animationFlagTable} - item={item + 2} - /> - </div> - ) - return ( - <RouteElement - key={item} - routeStatus={props.routeStatus} - tab={props.tab} - animationFlagTable={props.animationFlagTable} - item={item} - /> - ) - })} + {getFilteredIndices().map(item => + (item === 2 && props.routeStatus !== 'direct') + ? renderSpecialCase(item) + : renderStandardCase(item) + )} </> ) }src/app/components/routemap/DotAnimation.tsx (3)
143-149: Replace map with forEach when return value is not used.The
mapfunction is used here but its return value is ignored. This is confusing and doesn't follow best practices for array operations.Use
forEachinstead to make it clear that you're iterating for side effects:- currTimetableArray.map((currTimetable: SingleShuttleSchedule) => { + currTimetableArray.forEach((currTimetable: SingleShuttleSchedule) => { const dotAnimationConfig = getDotAnimationConfig(tab, currTimetable) dotAnimationConfig.forEach((config) => { tempFlagTable = applyDotAnimationFlag(config, tempFlagTable) }) })
69-114: Add validation for currTimetable.type.The function doesn't validate that
currTimetable.typeis a valid value before using it, which might lead to unexpected behavior.Consider adding validation or default handling for unexpected type values:
export const getDotAnimationConfig = ( tab: string, currTimetable: SingleShuttleSchedule, ) => { - if (currTimetable !== undefined) { + if (currTimetable !== undefined && currTimetable.type) { + // Validate the type is one of the expected values + const validTypes = ['C', 'DHJ', 'DY', 'D', 'NA']; + const type = validTypes.includes(currTimetable.type) ? currTimetable.type : 'D'; // Default to 'D' if invalid + switch (tab) { case 'shuttlecoke_o': - return [getDotAniTimetableConfig(currTimetable.type, 1)] + return [getDotAniTimetableConfig(type, 1)] case 'subway': - return [getDotAniTimetableConfig(currTimetable.type, 2)] + return [getDotAniTimetableConfig(type, 2)] case 'yesulin': - return [getDotAniTimetableConfig(currTimetable.type, 3)] + return [getDotAniTimetableConfig(type, 3)] case 'jungang': return [ { dotType: 'jungang', index: 3, }, ] case 'shuttlecoke_i': - if (currTimetable.type === 'NA') return [] + if (type === 'NA') return [] return [ { dotType: 'direct', index: 3, }, { dotType: 'jungang', index: 4, }, { dotType: 'cycle', index: 4, }, { dotType: 'yesulin', index: 4, }, ] default: - return [getDotAniTimetableConfig(currTimetable.type, 0)] + return [getDotAniTimetableConfig(type, 0)] } } return [] }
116-155: Add missing cleanup for tempFlagTable reference.The
useDotAnimationhook creates a new reference totempFlagTablefor each render but doesn't clean it up when the component unmounts, which might cause memory leaks.Add a cleanup function to the useEffect to ensure proper cleanup:
useEffect(() => { let tempFlagTable = { direct: [false, false, false, false, false, false], cycle: [false, false, false, false, false, false], yesulin: [false, false, false, false, false, false], jungang: [false, false, false, false, false, false], } setFlagTable(tempFlagTable) if (checkCurrTimetable.current === currTimetableArray[0]) return checkCurrTimetable.current = currTimetableArray[0] currTimetableArray.map((currTimetable: SingleShuttleSchedule) => { const dotAnimationConfig = getDotAnimationConfig(tab, currTimetable) dotAnimationConfig.forEach((config) => { tempFlagTable = applyDotAnimationFlag(config, tempFlagTable) }) }) setFlagTable(tempFlagTable) + + // Cleanup function + return () => { + tempFlagTable = { + direct: [false, false, false, false, false, false], + cycle: [false, false, false, false, false, false], + yesulin: [false, false, false, false, false, false], + jungang: [false, false, false, false, false, false], + } + } }, [currTimetableArray, tab])src/app/components/shuttle/Shuttle.tsx (1)
486-487: Add comments to clarify the purpose of timetable updates.The comments explain what the code does but not why. Consider adding context about how this relates to the route map and ping animation mentioned in the PR objectives.
- // Send filtered[0](or also include filtered[1] when bus arrive simultaneously) Array to RouteVisual - // when filtered has been updated. + // Send current timetable data to TimeTableContext to enable route map visualization + // and ping animation for active bus routes. If buses arrive simultaneously, include both.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/app/components/routemap/DotAnimation.tsx(1 hunks)src/app/components/routemap/RouteMap.tsx(1 hunks)src/app/components/routemap/RouteVisual.tsx(1 hunks)src/app/components/shuttle/Shuttle.tsx(6 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/app/components/routemap/RouteMap.tsx
🧰 Additional context used
🧬 Code Definitions (1)
src/app/components/shuttle/Shuttle.tsx (1)
src/app/context/TimeTableContext.tsx (1) (1)
useTimeTableContext(14-18)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Generate Lighthouse Report
- GitHub Check: Cloudflare Pages
🔇 Additional comments (6)
src/app/components/shuttle/Shuttle.tsx (6)
13-13: New context import supports timetable sharing.Good addition of the TimeTableContext import to enable sharing timetable data across components, which appears to be necessary for the route map feature mentioned in the PR objectives.
339-340: Improved code readability.Good refactoring to combine the conditional checks for 'R' and 'NA' chip types into a single condition, which makes the code more concise.
353-353: Context implementation allows timetable sharing.Good implementation of the TimeTableContext hook to access and update the shared timetable state across components.
565-565: Improved theme support with CSS variables.Good replacement of hardcoded color values with CSS variables for better theme support and consistency.
581-581: Improved theme support with CSS variables.Good replacement of hardcoded color values with CSS variables for better theme support and consistency.
392-402: Consider merging duplicate useEffect implementations.This useEffect handles error cases and empty timetable data, but there's a similar logic in the new code (lines 486-492). Consider refactoring to avoid duplication.
There was a problem hiding this comment.
Pull Request Overview
This pull request adds an interactive route map featuring dynamic animations to help users easily locate the shuttle bus and view active bus routes. Key changes include:
- Introduction of a new timetable context and accompanying hook to manage shuttle schedule data.
- Addition of DotAnimation functions and components to enable animated indicators in the route map.
- Integration of new RouteMap and RouteVisual components with corresponding updates in App.tsx and other UI modules.
Reviewed Changes
Copilot reviewed 29 out of 34 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/app/context/TimeTableContext.tsx | Adds a new context provider and hook for managing timetable data. |
| src/app/components/routemap/DotAnimation.tsx | Implements functions and components for dot animations. |
| src/app/components/routemap/RouteMap.tsx | Implements the new interactive route map layout. |
| src/app/components/routemap/RouteVisual.tsx | Enhances visualization of route elements and animation flags. |
| src/App.tsx | Integrates the RouteMap component and applies UI and transition tweaks. |
| src/app/components/shuttle/Shuttle.tsx | Updates chip color handling and integrates timetable state update. |
| src/app/components/subway/Subway.tsx | Fixes target window value and adjusts loading indicator props. |
| src/app/components/fulltime/FullTime.tsx | Adjusts UI layout and height calculations based on chip count. |
| src/app/components/modal/modalOpen.tsx | Refines changelog display and translation usage. |
| src/app/context/ThemeContext.tsx | Refines the theme selection logic based on local storage data. |
| src/app/components/useDarkMode.ts | Cleans up callback dependencies and formatting. |
| src/app/components/fab/fab.tsx | Updates styling for the floating action button. |
| src/app/data/index.ts | Exports new types for dot animation configuration. |
| src/app/components/lang/i18n.ts | Minor formatting updates in language initialization. |
Files not reviewed (5)
- .pnp.cjs: Language not supported
- package.json: Language not supported
- src/app/components/fab/fab.scss: Language not supported
- src/app/components/lang/lang.en.json: Language not supported
- src/app/components/lang/lang.ko.json: Language not supported
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/app/components/routemap/DotAnimation.tsx (1)
39-48:⚠️ Potential issueFix potential array index out-of-bounds issue
The current implementation contains potential index out-of-bounds issues when setting flags for different route types. For example, when
i >= 3andconfig.dotType === 'direct', the code tries to accessnewFlagArray.direct[i + 1]which might exceed the array bounds (especially at the last iteration wheni = config.index + 1).Consider adding bounds checking to all array accesses:
for (let i = config.index; i <= config.index + 1; i++) { if (config.dotType === 'direct') { - newFlagArray.direct[i >= 3 ? i + 1 : i] = true + const index = i >= 3 ? i + 1 : i; + if (index < newFlagArray.direct.length) { + newFlagArray.direct[index] = true; + } } - if (config.dotType === 'cycle') newFlagArray.cycle[i] = true + if (config.dotType === 'cycle' && i < newFlagArray.cycle.length) { + newFlagArray.cycle[i] = true; + } if (config.dotType === 'yesulin') { - newFlagArray.yesulin[i === 2 ? 3 : i] = true + const index = i === 2 ? 3 : i; + if (index < newFlagArray.yesulin.length) { + newFlagArray.yesulin[index] = true; + } } - if (config.dotType === 'jungang') newFlagArray.jungang[i] = true + if (config.dotType === 'jungang' && i < newFlagArray.jungang.length) { + newFlagArray.jungang[i] = true; + } }
🧹 Nitpick comments (6)
src/app/context/TimeTableContext.tsx (1)
12-12: Consider initializing with a non-null default value.While initializing with
nullworks since you check for it in the hook, consider providing a default empty implementation to avoid the need for null checks.-const TimeTableContext = React.createContext<TimeTableContextProps | null>(null) +const TimeTableContext = React.createContext<TimeTableContextProps>({ + currTimetable: [{ type: 'NA', time: '' }], + setCurrTimetable: () => {}, +})src/app/components/shuttle/Shuttle.tsx (1)
507-508: Consider adding JSDoc comments for the RouteVisual feature.The comments mention sending data to a RouteVisual component, but this component's purpose and implementation aren't immediately clear from the context.
-// Send filtered[0](or also include filtered[1] when bus arrive simultaneously) Array to RouteVisual -// when filtered has been updated. +/** + * Updates the TimeTableContext with the latest bus information + * This data will be consumed by the RouteMap component to visualize + * the active route with a ping animation as mentioned in the PR objectives + * @param {Array} filtered - Filtered array of upcoming buses + */src/app/components/routemap/DotAnimation.tsx (4)
143-149: Consider using object spread operator for all arrays in a single updateWhen mapping through multiple timetables, the code is updating the temporary flag table with each iteration. This approach can lead to potential race conditions if the state of the flag array changes during the async operations.
Consider using
reducefor a more functional approach:- currTimetableArray.map((currTimetable: SingleShuttleSchedule) => { - const dotAnimationConfig = getDotAnimationConfig(tab, currTimetable) - - dotAnimationConfig.forEach((config) => { - tempFlagTable = applyDotAnimationFlag(config, tempFlagTable) - }) - }) + const tempFlagTable = currTimetableArray.reduce((accFlags, currTimetable) => { + const configs = getDotAnimationConfig(tab, currTimetable); + return configs.reduce((flags, config) => { + return applyDotAnimationFlag(config, flags); + }, accFlags); + }, { + direct: [false, false, false, false, false, false], + cycle: [false, false, false, false, false, false], + yesulin: [false, false, false, false, false, false], + jungang: [false, false, false, false, false, false], + });
163-163: Add return value to early return statementWhen the route status is 'yesulin' and the index is 2, the component doesn't return anything explicitly.
- if (props.routeStatus === 'yesulin' && props.index === 2) return + if (props.routeStatus === 'yesulin' && props.index === 2) return null
41-45: Ensure code formatting and style consistencyThe conditional statements are formatted inconsistently - some have curly braces, some are on a single line without braces. This can lead to bugs and maintenance issues.
- if (config.dotType === 'direct') - newFlagArray.direct[i >= 3 ? i + 1 : i] = true - if (config.dotType === 'cycle') newFlagArray.cycle[i] = true - if (config.dotType === 'yesulin') - newFlagArray.yesulin[i === 2 ? 3 : i] = true - if (config.dotType === 'jungang') newFlagArray.jungang[i] = true + if (config.dotType === 'direct') { + newFlagArray.direct[i >= 3 ? i + 1 : i] = true + } + if (config.dotType === 'cycle') { + newFlagArray.cycle[i] = true + } + if (config.dotType === 'yesulin') { + newFlagArray.yesulin[i === 2 ? 3 : i] = true + } + if (config.dotType === 'jungang') { + newFlagArray.jungang[i] = true + }
161-162: Consider using TypeScript enum or union type for route statusThe
routeStatusprop is currently typed as a string, but it appears to have specific allowed values like 'yesulin'.Consider creating a union type for better type safety:
type RouteStatus = 'direct' | 'cycle' | 'yesulin' | 'jungang'; // Then update the props type: const DotAnimation = (props: { isOn: boolean index: number color: string routeStatus: RouteStatus }) => { // ... }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/app/components/routemap/DotAnimation.tsx(1 hunks)src/app/components/shuttle/Shuttle.tsx(10 hunks)src/app/context/TimeTableContext.tsx(1 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
src/app/components/shuttle/Shuttle.tsx (1)
src/app/context/TimeTableContext.tsx (1) (1)
useTimeTableContext(14-18)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Cloudflare Pages
🔇 Additional comments (13)
src/app/context/TimeTableContext.tsx (4)
1-4: Good modular architecture with a dedicated context.The creation of a separate context file for timetable management is a good architectural decision that will improve code organization and state management across components.
5-10: Clean interface definition for type safety.The
TimeTableContextPropsinterface clearly defines the shape of the context with proper typing, making the code more maintainable and less prone to type errors.
14-18: Good error handling in the context hook.The error thrown when context is not available provides clear guidance for developers, improving debugging experience.
20-37: Well-implemented context provider with performance optimization.The context provider follows best practices by:
- Properly initializing state with a default value
- Using
useMemoto optimize re-renders- Following the React children pattern
This addresses the past review comment concern about null values in the timetable.
src/app/components/shuttle/Shuttle.tsx (6)
13-13: Good integration of the new context.The import and use of the context hook shows a clean integration with the new context system.
339-340: Consistent styling for 'R' and 'NA' chip types.Grouping these types with the same styling creates a consistent visual treatment.
353-353: Effective state management with context.Using the context hook to access shared state improves component communication and reduces prop drilling.
449-451: Consistent error handling across multiple scenarios.The code properly resets the timetable context to a default value in various error cases, ensuring the UI stays in a consistent state.
Also applies to: 459-461, 478-480, 494-496
506-512: Smart context update logic for route visualization.The code intelligently updates the context based on:
- Whether the data has changed
- Whether simultaneous buses are arriving
- Only when necessary to avoid unnecessary rerenders
This supports the PR's objective of adding route map visualization.
573-574: Improved theming support with CSS variables.Replacing hardcoded colors with CSS variables enhances theme support and dark mode compatibility.
Also applies to: 589-594
src/app/components/routemap/DotAnimation.tsx (3)
1-11: LGTM! Imports are clean and organizedThe imports are well-structured, with React and styling-related imports grouped at the top, followed by internal app imports.
12-15: Good use of styled-components with twin.macro for responsive stylingThe
PingDotcomponent nicely leverages the power of styled-components with twin.macro, including responsive styling with thert1:prefix for different screen sizes.
17-28: Excellent documentation for applyDotAnimationFlag functionThe JSDoc comments clearly explain the purpose of the function, parameters, return value, and even include a concrete example of how it works. This is excellent for maintainability.
|
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/routemap/DotAnimation.tsx (1)
39-46:⚠️ Potential issueAdd bounds checking for array indices to prevent out-of-bounds access.
The array index manipulations in the
applyDotAnimationFlagfunction lack bounds checking, which could lead to out-of-bounds access and potential runtime errors.Consider implementing bounds checking for all routes:
for (let i = config.index; i <= config.index + 1; i++) { if (config.dotType === 'direct') { - newFlagArray.direct[i >= 3 ? i + 1 : i] = true + const index = i >= 3 ? i + 1 : i + if (index < newFlagArray.direct.length) { + newFlagArray.direct[index] = true + } } if (config.dotType === 'cycle') { - newFlagArray.cycle[i] = true + if (i < newFlagArray.cycle.length) { + newFlagArray.cycle[i] = true + } } if (config.dotType === 'yesulin') { - newFlagArray.yesulin[i === 2 ? 3 : i] = true + const index = i === 2 ? 3 : i + if (index < newFlagArray.yesulin.length) { + newFlagArray.yesulin[index] = true + } } if (config.dotType === 'jungang') { - newFlagArray.jungang[i] = true + if (i < newFlagArray.jungang.length) { + newFlagArray.jungang[i] = true + } } }
🧹 Nitpick comments (4)
src/app/components/routemap/DotAnimation.tsx (4)
116-154: Optimize state management in the useDotAnimation hook.The implementation of the
useDotAnimationhook has been improved, but there are still some optimizations that could be made to prevent unnecessary re-renders and improve efficiency.
- Consider using a callback for the
setFlagTablestate update to ensure it works with the latest state:useEffect(() => { if (checkCurrTimetable.current === currTimetableArray[0]) return checkCurrTimetable.current = currTimetableArray[0] let tempFlagTable = { direct: [false, false, false, false, false, false], cycle: [false, false, false, false, false, false], yesulin: [false, false, false, false, false, false], jungang: [false, false, false, false, false, false], } currTimetableArray.forEach((currTimetable: SingleShuttleSchedule) => { const dotAnimationConfig = getDotAnimationConfig(tab, currTimetable) dotAnimationConfig.forEach((config) => { tempFlagTable = applyDotAnimationFlag(config, tempFlagTable) }) }) - setFlagTable(tempFlagTable) + setFlagTable(() => tempFlagTable) }, [currTimetableArray, tab])
- Consider using
useMemoto memoize the route configurations to prevent recalculations when the dependencies haven't changed:const dotConfigurations = React.useMemo(() => { return currTimetableArray.flatMap((currTimetable: SingleShuttleSchedule) => { return getDotAnimationConfig(tab, currTimetable) }) }, [currTimetableArray, tab]) useEffect(() => { if (checkCurrTimetable.current === currTimetableArray[0]) return checkCurrTimetable.current = currTimetableArray[0] let tempFlagTable = { direct: [false, false, false, false, false, false], cycle: [false, false, false, false, false, false], yesulin: [false, false, false, false, false, false], jungang: [false, false, false, false, false, false], } dotConfigurations.forEach((config) => { tempFlagTable = applyDotAnimationFlag(config, tempFlagTable) }) setFlagTable(() => tempFlagTable) }, [currTimetableArray, tab, dotConfigurations])
162-164: Improve the conditional rendering logic for better code clarity.The early return for a specific condition makes the component logic harder to follow. Consider using a more explicit conditional rendering approach.
const DotAnimation = (props: { isOn: boolean index: number color: string routeStatus: string }) => { - if (props.routeStatus === 'yesulin' && props.index === 2) return - - return <PingDot className={props.color} $on={props.isOn} /> + return ( + (props.routeStatus !== 'yesulin' || props.index !== 2) && + <PingDot className={props.color} $on={props.isOn} /> + ) }
73-112: Consider extracting the case-specific logic to improve maintainability.The
getDotAnimationConfigfunction has a large switch statement with nested logic, making it harder to maintain as the number of cases grows.Consider extracting each case into a separate function to improve readability and maintainability:
const getShuttlecokeOConfig = (currTimetable: SingleShuttleSchedule) => { return [getDotAniTimetableConfig(currTimetable.type, 1)] } const getSubwayConfig = (currTimetable: SingleShuttleSchedule) => { return [getDotAniTimetableConfig(currTimetable.type, 2)] } // ... other case-specific functions export const getDotAnimationConfig = ( tab: string, currTimetable: SingleShuttleSchedule, ) => { if (currTimetable.time === '') return [] switch (tab) { case 'shuttlecoke_o': return getShuttlecokeOConfig(currTimetable) case 'subway': return getSubwayConfig(currTimetable) // ... other cases default: return [getDotAniTimetableConfig(currTimetable.type, 0)] } }
156-167: Consider adding PropTypes or TypeScript interface for better component documentation.While the component is using TypeScript for typing, it would be beneficial to extract the props interface to make it more reusable and to improve documentation.
+interface DotAnimationProps { + isOn: boolean + index: number + color: string + routeStatus: string +} + -const DotAnimation = (props: { - isOn: boolean - index: number - color: string - routeStatus: string -}) => { +const DotAnimation = (props: DotAnimationProps) => { if (props.routeStatus === 'yesulin' && props.index === 2) return return <PingDot className={props.color} $on={props.isOn} /> }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/app/components/routemap/DotAnimation.tsx(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Cloudflare Pages
|
@coderabbitai resolve |
✅ Actions performedComments resolved. |
A Friendly RouteMap
Additional content
Any additional comments? 😁
Summary by CodeRabbit
New Features
Style
Bug Fixes