Add Route Map #99
Add Route Map #99
Conversation
…te Shuttlecoke_i color
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⛔ Files ignored due to path filters (5)
📒 Files selected for processing (16)
Tip You can generate walkthrough in a markdown collapsible section to save space.Enable the 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Deploying hybus-genesis with
|
| Latest commit: |
cf87f1e
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://fa189d1a.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: 2
♻️ Duplicate comments (2)
src/app/components/fulltime/FullTime.tsx (1)
294-294:⚠️ Potential issueMissing key property in iterable.
This empty return fragment needs a unique key since it's in an iterable.
- return <></> + return <React.Fragment key={hour}><></></React.Fragment>🧰 Tools
🪛 Biome (1.9.4)
[error] 294-294: 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)
src/app/components/shuttle/Shuttle.tsx (1)
424-430: 🛠️ Refactor suggestionConsider handling empty arrays.
Currently, if no valid entries exist,filtered[0]will beundefined. Safeguard against potential side effects:useEffect(() => { if (timetable.data !== undefined) { const filtered = timetable.data.filter((val) => isAfterCurrentTime(val)) - setTimetable(filtered[0]) + if (filtered.length > 0) { + setTimetable(filtered[0]) + } else { + setTimetable(undefined) + } } }, [currentTime, setTimetable, timetable.data])
🧹 Nitpick comments (9)
src/app/data/routemap/circleanimate.ts (1)
3-7: Add documentation to explain the purpose of this type.This type appears to be for managing animated circle components, but lacks documentation explaining its purpose and usage. Consider adding JSDoc comments to make the code more maintainable.
+/** + * Type definition for circle animation components. + * Used to control animated circles in the route map visualization. + */ export type CircleAnimate = { + /** Reference to HTML div elements that represent the circles */ ref: React.MutableRefObject<Array<HTMLDivElement>>, + /** Index position of the circle in the sequence */ index: number, + /** Color of the circle, used for styling */ chipColor: string, }src/app/components/lang/lang.en.json (1)
12-14: Remove unnecessary blank line.There's an extra blank line after the new translations that should be removed to maintain consistent formatting throughout the JSON file.
"yesul": "Yesulin", "jung": "Jungang", - "NA": "N/A",src/app/context/TimeTableContext.tsx (3)
12-16: Improve error message in useTimeTableContext hook.The current error message could be more descriptive to help developers understand the issue better.
export const useTimeTableContext = () => { const context = React.useContext(TimeTableContext) - if (!context) throw Error('TimeTable provider not defined!') + if (!context) throw Error('useTimeTableContext must be used within a TimeTableContextProvider!') return context }
5-27: Add JSDoc comments to the context implementation.Adding JSDoc comments would improve the code's maintainability and make it easier for other developers to understand the purpose and usage of this context.
+/** + * Interface defining the shape of the TimeTable context. + */ interface TimeTableContextProps { timetable: SingleShuttleSchedule | null, setTimetable: React.Dispatch<React.SetStateAction<SingleShuttleSchedule | null>> } +/** + * Context for managing and sharing timetable data across components. + */ const TimeTableContext = React.createContext<TimeTableContextProps | null>(null) +/** + * Hook to access the TimeTable context. + * @returns The TimeTable context value + * @throws Error if used outside of a TimeTableContextProvider + */ export const useTimeTableContext = () => { const context = React.useContext(TimeTableContext) if (!context) throw Error('TimeTable provider not defined!') return context } +/** + * Provider component for the TimeTable context. + * Manages the timetable state and provides it to child components. + */ export const TimeTableContextProvider = ({ children, }: React.PropsWithChildren) => {
10-10: Consider providing default values for the context.Instead of initializing the context with null, consider providing default values to avoid having to check for null when consuming the context.
-const TimeTableContext = React.createContext<TimeTableContextProps | null>(null) +const TimeTableContext = React.createContext<TimeTableContextProps>({ + timetable: null, + setTimetable: () => { + console.warn('TimeTableContext: setTimetable was called without a provider') + } +})This would also allow you to remove the null check in the
useTimeTableContexthook.src/App.tsx (1)
448-478: Well-structured Transition block.
Animating the route-card expansion enhances the user experience. Consider handling interim states (entering,exiting) if you want finer control over partial animations.src/app/components/routemap/RouteMap.tsx (3)
102-173: Large conditional logic in directInput.
Constructing multiple arrays in a loop works here, but consider extracting smaller helper functions to reduce complexity.
175-212: Similar pattern in directLineInput.
This is well-structured for line creation, though you could unify repeated code with helper utilities shared withdirectInput.
306-341: Highlight logic in useEffect.
A large if-else chain forprops.tabcan be replaced with a lookup table for maintainability.
📜 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 (16)
.pnp.cjs(8 hunks)package.json(2 hunks)src/App.tsx(5 hunks)src/app/components/fulltime/FullTime.tsx(6 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/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/shuttle.ts(1 hunks)src/index.css(4 hunks)src/main.tsx(2 hunks)tailwind.config.js(4 hunks)
🧰 Additional context used
🪛 GitHub Check: Generate Lighthouse Report
src/app/data/index.ts
[failure] 5-5:
Cannot find module './routemap/circleAnimate' or its corresponding type declarations.
🪛 GitHub Actions: Lighthouse CI
src/app/data/index.ts
[error] 5-5: error TS2307: Cannot find module './routemap/circleAnimate' or its corresponding type declarations.
🪛 Biome (1.9.4)
src/app/components/fulltime/FullTime.tsx
[error] 294-294: 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 (49)
src/app/network/shuttle.ts (1)
16-16:❓ Verification inconclusive
API endpoint version update
The API endpoint has been updated from v1 to v2 for the timetable resource. This change indicates a version upgrade in the backend API.
Make sure that the v2 endpoint is properly deployed and accessible in all environments. Consider documenting the changes between v1 and v2 if there are significant differences in response structure.
🏁 Script executed:
#!/bin/bash # Check if v2 endpoint is referenced elsewhere in the codebase rg -g "*.{ts,tsx,js,jsx}" "\/v2\/timetable" --no-filename | wc -lLength of output: 151
API Endpoint Version Update – Verification and Documentation Needed
- The endpoint now uses
/v2/timetable/${season}/${week}/${location}reflecting the API upgrade.- Please verify that the
/v2/timetableendpoint is deployed and accessible across all environments.- Note: The verification script encountered a regex error due to unnecessary escape sequences. To correctly search for endpoint references, use a query like:
rg -g "*.{ts,tsx,js,jsx}" "/v2/timetable"- If there are significant changes in the response structure between v1 and v2, please document these differences.
package.json (2)
34-34: Added animation dependencyAdding
react-transition-groupfor UI animations is a good choice for implementing smooth transitions.This library is well-maintained and widely used for animation effects in React applications.
44-44: TypeScript type definitions for animationsGood practice to include TypeScript type definitions for the newly added
react-transition-grouplibrary.This ensures proper type checking and IDE autocompletion support when using the animation components.
src/app/data/routemap/circleanimate.ts (1)
1-7: File naming should follow lowerCamelCase convention.The file name should be
circleAnimate.tsinstead ofcircleanimate.tsto match the lowerCamelCase pattern used in React/TypeScript projects.src/main.tsx (2)
44-44: Appropriate z-index adjustment.The z-index has been correctly set to 2 as suggested in the previous review, which positions the cherry blossom effect above the Circle component (z-index 1) and Line component (z-index 0).
50-52: TimeTableContextProvider added appropriately.The TimeTableContextProvider has been correctly implemented to wrap the App component, allowing timetable state to be accessed throughout the application.
src/app/components/lang/lang.en.json (1)
12-13: Appropriate new translations added.The new localization keys for "yesul", "jung", and "dorm" have been correctly added to support the route map feature.
Also applies to: 25-25
src/app/context/TimeTableContext.tsx (1)
1-27: TimeTableContext implementation looks good.The context implementation follows React best practices with proper typing and error handling.
src/app/components/lang/lang.ko.json (3)
12-13: Added new location translations.The addition of "yesul" (예술인) translation aligns with the new route map feature and enhances the application's localization support.
14-14: Added central station translation.Adding "jung" (중앙역) provides proper Korean localization for the central station location in the route map.
25-25: Added dormitory translation.The addition of "dorm" (기숙사) translation completes the set of new location references needed for the route map feature.
tailwind.config.js (4)
14-15: Added responsive breakpoints for wider screens.The new
wide(455px) andmwide(430px) breakpoints will improve the responsive layout for the route map on larger mobile devices.
35-35: Added arrow color theming support.This new color variable connects to the CSS custom property defined in index.css, ensuring consistent arrow styling across themes.
47-47: Added orange chip color for shuttle indicators.The
chip-orangecolor (#FFAB40) will provide visual distinction for shuttle routes in the route map interface.
146-148: Added height transition property.This addition will enable smooth height animations when expanding or collapsing route information, improving the user experience.
src/app/components/fulltime/FullTime.tsx (9)
103-105: Added height configuration for four chips display.Good addition to support the new route type. The height settings are proportionally adjusted for both standard and mobile views.
175-178: Added display support for Yesulin routes.The implementation correctly displays Yesulin (예술인) routes with a green chip styling and proper translation lookup.
180-180: Added conditional styling for shuttle routes.The ternary operator appropriately applies orange background for shuttle routes (isShuttleI) or blue for other direct routes.
183-192: Fixed potential duplicate route display.Good implementation that prevents routes from being displayed twice when they appear in both direct and directY arrays.
However, there's a minor optimization opportunity:
- 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> )
257-257: Added shuttle route indicator property.The new boolean property tracks shuttle route types appropriately.
274-276: Added logic to detect shuttle route type.This logic correctly identifies "R" type schedules as shuttle routes and sets the flag accordingly.
288-290: Updated counter for Yesulin routes.Good addition to count Yesulin routes for proper height calculation in the TimeBox component.
312-312: Updated conditional rendering check.The condition now correctly accounts for all route types including the new Yesulin and Jungang routes.
319-319: Added shuttle indicator to TimeBox props.Properly passing the isShuttleI flag to the TimeBox component ensures correct styling of shuttle routes.
src/index.css (1)
31-31: Added arrow color theming across all themes.Great attention to detail by adding the arrow color variables to all four themes (light, dark, Christmas, and spring). This ensures consistent styling of directional indicators in the route map across different theme options.
Also applies to: 55-55, 79-79, 103-103
.pnp.cjs (8)
31-37: New Dependency for @types/react-transition-group Added
The new entry for@types/react-transition-group(version4.4.11) is correctly added. Please verify that the version remains consistent with your other dependency files (e.g. package.json) and that its placement in this section fits your resolution strategy.
63-69: Addition of react-transition-group Dependency
The dependency forreact-transition-group(version4.4.5) is introduced here with a virtual resolution. Ensure that this version aligns properly with the corresponding type definitions and any UI animation logic that depends on it.
2729-2742: Updated Babel Runtime and regenerator-runtime Block
The restructuring of the dependency block for@babel/runtimeandregenerator-runtimeis clear and well formatted. Please confirm that the new structure does not inadvertently affect module resolution or conflicting versions within your runtime environment.
4121-4134: New Block for @types/react-transition-group in Resolution Mapping
A dedicated dependency block for@types/react-transition-grouphas been added with explicit package location and dependencies. Verify that the version (4.4.11) here remains in sync with the entry elsewhere in the file and that it meets the TypeScript requirements of your project.
5793-5809: Addition of dom-helpers Dependency Block
The dependency block fordom-helpers(version5.2.1) is properly introduced along with its package location and dependencies (@babel/runtimeandcsstype). Please double-check that all transitive dependencies are compatible with your project’s configuration.
7229-7235: Duplicate Entry for @types/react-transition-group in Dependency Block
This section again lists@types/react-transition-group(version4.4.11), which might be intentional for different resolution contexts. Please verify that this apparent duplication is expected and does not cause version conflicts in your dependency graph.
7261-7267: Repeated Dependency Block Including react-transition-group
Similar to the earlier block, this section reintroducesreact-transition-group(version4.4.5) alongside other dependencies. Confirm that the repetition across these resolution blocks is deliberate and that consistent versions are maintained throughout.
9544-9579: Dual Entry for react-transition-group with Different Link Types
A new dependency block forreact-transition-groupnow appears with both “SOFT” and “HARD” link types. This dual declaration might be intended to support different usage contexts (e.g. a virtual versus a normal resolution), but please double-check that both definitions are necessary and will not lead to conflicts during module resolution.src/app/components/shuttle/Shuttle.tsx (3)
13-13: Great addition of the TimeTableContext import.
This import opens up the ability to manage timetable state directly from context, enhancing reusability and reducing prop drilling.
339-341: Consistent styling for 'R' and 'NA' chip types.
Displaying both 'R' and 'NA' as orange chips maintains clarity in the UI. Ensure that this color mapping correctly reflects your product requirements.
353-355: Instantiation of setTimetable looks good.
UsingsetTimetablein this component is a straightforward approach for updating the stored timetable upon relevant UI changes.src/App.tsx (9)
5-5: Smooth transition import.
Bringing inTransitioncleanly integrates animated UI flows.
10-10: Importing Arrow asset.
No issues found with SVG usage for toggling.
14-14: RouteMap import is correctly placed.
This import prepares for dynamic route mapping.
19-19: Refreshing component import.
Straightforward addition for a pull-to-refresh UI.
121-123: Height transition based on state.
Providing different heights for'entered'vs. default states streamlines the open/closed animation.
130-134: Opacity toggling for entered/exited states.
This is a valid pattern, but confirm that showing/hiding the container with contradictory rules (hiddenvs.opacity-100) is intended.
135-138: Arrow rotation is handled elegantly.
Rotating between 0 and 180 degrees is a neat approach for a toggle indicator.
162-162: Elevating z-index for consistent layering.
This helps ensure interactive elements remain on top where needed.
199-199: New routeCardClick state.
Clean approach to track toggle behavior for the route card.src/app/components/routemap/RouteMap.tsx (4)
72-75: Explicit component props.
Definingstatusandtabclarifies usage. Consider adding a short doc comment describing expected values for each.
214-233: Responsive line-drawing logic.
Calculates widths correctly to visually connect route stations. Implementation appears consistent.
254-271: Circle animation logic.
Appending the ping circle is intuitive. Double-check thati === 5 || (i === 4 && props.chipColor === 'bg-chip-blue')won’t cause unintentional duplication.
343-391: Overall route layout rendered effectively.
Displays multiple route segments with separate station lines.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
src/app/data/index.ts (1)
5-5:⚠️ Potential issueResolve file name casing issue
There's a casing inconsistency between the imported file and the actual file name. The TypeScript compiler is reporting that the file name differs only in casing (
circleAnimate.tsvscircleanimate.ts).This is causing the build pipeline to fail with error:
error TS1261: Already included file name differs from file name only in casing.To fix this issue, ensure that the file name casing is consistent between the import statement and the actual file:
- Rename the file to match the casing in the import statement, or
- Update the import statement to match the actual file name casing
#!/bin/bash # Check the actual file name in the file system find src/app/data/routemap -type f | grep -i circle🧰 Tools
🪛 GitHub Check: Generate Lighthouse Report
[failure] 5-5:
Already included file name '/home/runner/work/-genesis/-genesis/src/app/data/routemap/circleAnimate.ts' differs from file name '/home/runner/work/-genesis/-genesis/src/app/data/routemap/circleanimate.ts' only in casing.🪛 GitHub Actions: Lighthouse CI
[error] 5-5: error TS1261: Already included file name differs from file name only in casing.
src/app/data/routemap/circleanimate.ts (1)
1-7:⚠️ Potential issueType definition looks good, but the file name should follow lowerCamelCase pattern.
The
CircleAnimatetype is well-structured with appropriate types for each property. However, the file name should becircleAnimate.tsinstead ofcircleanimate.tsto follow the TypeScript/JavaScript lowerCamelCase convention.src/app/components/shuttle/Shuttle.tsx (1)
424-429:⚠️ Potential issueAdd error handling for empty filtered bus data
The new effect properly filters and sets the timetable, but doesn't handle the case where no buses meet the filtering criteria, which could pass undefined values to the context.
useEffect(() => { if(timetable.data !== undefined){ const filtered = timetable.data.filter((val) => isAfterCurrentTime(val)) - setTimetable(filtered[0]) + if (filtered.length > 0) { + setTimetable(filtered[0]) + } } }, [currentTime, setTimetable, timetable.data])
🧹 Nitpick comments (8)
src/app/data/routemap/circleAnimate.ts (1)
1-7: Clean up code style for consistencyThe new type definition looks good functionally, but there are a few style issues to address:
- Remove unnecessary trailing commas after properties
- Remove the semicolon after the import statement for consistency
- Ensure the file name casing matches the import in
src/app/data/index.ts-import React from 'react'; +import React from 'react' export type CircleAnimate = { ref: React.MutableRefObject<Array<HTMLDivElement>>, index: number, - chipColor: string, + chipColor: string }Also, ensure that the file name follows proper casing conventions. The current import in
index.tssuggests the file should be namedcircleAnimate.ts, but there appears to be another file with the namecircleanimate.ts. This casing inconsistency is causing build failures.src/app/context/TimeTableContext.tsx (1)
14-14: Consider providing more descriptive error message.The error message could be more specific to help with debugging.
- if (!context) throw Error('TimeTable provider not defined!') + if (!context) throw Error('useTimeTableContext must be used within a TimeTableContextProvider')src/app/components/lang/lang.ko.json (1)
24-25: Added trailing space and new "dorm" keyThere's a trailing space added to the "residence" value, which appears to be unintentional. Also, the addition of the "dorm" key is consistent with the English localization.
- "residence": "기숙사 (셔틀콕행)", + "residence": "기숙사 (셔틀콕행)",tailwind.config.js (1)
14-15: Consider consistent ordering for newly added breakpoints.
Adding these custom breakpoints looks good. For clarity and maintainability, consider placing them in a logical sequence (e.g., smallest to largest) if you anticipate more breakpoints in the future.src/app/components/fulltime/FullTime.tsx (2)
103-104: Suggest switch-case or mapping approach for more than four$maxChipsconditions.
While adding a fourth condition is workable, consider refactoring into a more scalable structure if you foresee additional$maxChipsvalues in the future.if ($maxChips === 1) { return tw`h-24 hm:h-20` } else if ($maxChips === 2) { return tw`h-28 hm:h-24` } else if ($maxChips === 3) { return tw`h-32 hm:h-28` - } else if ($maxChips === 4) { - return tw`h-40 hm:h-32` + } else { + return tw`h-40 hm:h-32` }
180-192: Refactor.map()usage for clarity and efficiency.
Using.map()just to setisExistcan be less readable. Consider a.some()approach or using aSetfor lookups to simplify:{props.direct.map((time, idx) => { - let isExist = false - props.directY.map((ytime) => { - time === ytime ? (isExist = true) : null - }) - return( - isExist ? null : - <React.Fragment key={idx}> + const isExist = props.directY.some((ytime) => ytime === time) + return isExist ? null : ( + <React.Fragment key={idx}> <span>{time} </span> </React.Fragment> ) })}src/app/components/routemap/RouteMap.tsx (2)
294-338: Simplify tab-based conditionals.
The largeif-elsechain insideupdateHighlight()forprops.tabcan be simplified to a dictionary-based approach or a well-structured switch statement. This would clean up the branching logic and make further additions easier.
343-392: Consider abstracting repeated JSX blocks.
The repeated patterns for direct, cycle, yesulin, and jungang route sections share a similar structure. Extracting them into smaller sub-components or mapping logic can reduce duplication and expedite modifications or styling changes in the future.
📜 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 (17)
.pnp.cjs(8 hunks)package.json(2 hunks)src/App.tsx(5 hunks)src/app/components/fulltime/FullTime.tsx(6 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/context/TimeTableContext.tsx(1 hunks)src/app/data/index.ts(1 hunks)src/app/data/routemap/circleAnimate.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/index.css(4 hunks)src/main.tsx(2 hunks)tailwind.config.js(4 hunks)
🧰 Additional context used
🪛 GitHub Check: Generate Lighthouse Report
src/app/data/index.ts
[failure] 5-5:
Already included file name '/home/runner/work/-genesis/-genesis/src/app/data/routemap/circleAnimate.ts' differs from file name '/home/runner/work/-genesis/-genesis/src/app/data/routemap/circleanimate.ts' only in casing.
🪛 GitHub Actions: Lighthouse CI
src/app/data/index.ts
[error] 5-5: error TS1261: Already included file name differs from file name only in casing.
🪛 Biome (1.9.4)
src/app/components/fulltime/FullTime.tsx
[error] 294-294: 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 (46)
src/app/network/shuttle.ts (1)
16-16:❓ Verification inconclusive
API endpoint version has been updated
The API endpoint has been updated to use a versioned path (
/v2/timetable/...). This change indicates a migration to a newer version of the API endpoint.Please verify that:
- The v2 endpoint is properly implemented and available on the server side
- The response structure from this endpoint is compatible with the existing code that processes the response
- There is appropriate error handling if the v2 endpoint is not available
🏁 Script executed:
#!/bin/bash # Check if there are other references to the timetable API endpoint that might need to be updated rg -g "*.{ts,tsx}" "\/timetable\/" --no-filenameLength of output: 130
API v2 Endpoint Verification Required
The code now uses the updated
/v2/timetable/${season}/${week}/${location}template, which suggests a migration to a new API version. Before approving this change, please verify the following manually:
- Server-Side Implementation: Confirm that the
/v2/timetable/...endpoint is correctly implemented and is live on the server.- Response Structure Consistency: Ensure that the response format from this new endpoint is compatible with the existing client-side processing.
- Error Handling: Verify that there is robust error handling in place when the v2 endpoint is unavailable.
Additional Note:
An earlier attempt to search for timetable endpoint references using the commandrg -g "*.{ts,tsx}" "\/timetable\/" --no-filenameresulted in a regex parse error (unrecognized escape sequence). Please re-run the search with a corrected query, for example:
#!/bin/bash # Corrected RG query to search for references to the updated timetable endpoint rg -g '*.{ts,tsx}' '/v2/timetable/' --no-filenameIf further investigation is needed beyond this re-run, additional manual verification or targeted searches may be required.
src/app/data/shuttle/organizedTimetables.ts (1)
6-7: Make sure to initialize the new properties in all instancesThe
OrganizedTimetablestype has been extended with thejungangarray property andisShuttleIboolean property. This change will affect all places where this type is used.Ensure that both properties are properly initialized whenever an
OrganizedTimetablesobject is created throughout the codebase:#!/bin/bash # Find all places where OrganizedTimetables objects are created rg -g "*.{ts,tsx}" "OrganizedTimetables" -A 3 -B 3For example, in
src/app/components/fulltime/FullTime.tsxand any other components that create objects of this type, update the initialization to include these properties:const timetableObj: OrganizedTimetables = { time: hour, direct: [], circle: [], // if this exists directY: [], // if this exists jungang: [], isShuttleI: false, // or appropriate default value count: 0 // if this exists }src/main.tsx (2)
44-44: zIndex value change is appropriate.The zIndex change from 1 to 2 properly implements the z-index hierarchy discussed in previous comments, positioning the cherry blossom effect above the Circle component.
50-52: Good addition of the TimeTableContextProvider.Adding the TimeTableContextProvider here makes the timetable context accessible to all components within the App, which is a good architectural decision for sharing timetable data across components.
package.json (2)
34-34: Good addition of react-transition-group dependency.Adding the
react-transition-grouppackage is appropriate for implementing animated transitions, which aligns with the Route Map feature being added.
44-44: TypeScript type definitions are properly included.Including the corresponding TypeScript type definitions for
react-transition-groupis a good practice that will provide proper type checking and IDE support.src/app/context/TimeTableContext.tsx (1)
1-27: Well-structured context implementation for timetable management.The TimeTableContext is implemented with best practices:
- Clear interface definition
- Proper null checking in the hook
- Clean provider implementation with useState
- Good error handling for when the context is used outside its provider
This will provide a solid foundation for sharing timetable data across components.
src/app/components/lang/lang.en.json (2)
12-13: New localization keys added for locationsThe addition of shorter-form keys "yesul" and "jung" alongside the existing "yesulin" and "jungang" keys provides more flexibility for different UI contexts. This is a good improvement for localization.
25-25: Added "dorm" as shorter alternative to "residence"The new "dorm" key provides a more concise label option compared to the existing "residence" key. This will be useful for space-constrained UI elements.
src/app/components/shuttle/Shuttle.tsx (3)
13-13: New context hook imported for timetable managementThe import of
useTimeTableContextaligns with the implementation of a centralized timetable state management approach.
339-341: Added styling for R and NA bus typesThe orange chip styling for 'R' and 'NA' bus types improves visual consistency. Good enhancement to the UI.
354-355: Using timetable context for state managementExtracting
setTimetablefrom the context hook indicates a move toward centralized state management.src/app/components/lang/lang.ko.json (1)
12-14: New Korean localization keys addedThe addition of "yesul" and "jung" keys with their appropriate Korean translations maintains consistency with the English localization file and improves the user experience for Korean speakers.
src/index.css (1)
31-31:✅ Verification successful
Added arrow color variables across all themes
The addition of
--color-arrow-colorCSS variables for all themes (light, dark, Christmas, and spring) ensures consistent styling of arrow elements throughout the application. This is a good practice for maintaining theme consistency.Also applies to: 55-55, 79-79, 103-103
🏁 Script executed:
#!/bin/bash # Check if the arrow-color is used in the Tailwind config grep -r "arrow-color" --include="*.js" --include="*.ts" --include="*.tsx" .Length of output: 260
Arrow Color Variable Usage Verified and Approved
The inclusion of the
--color-arrow-colorCSS variable has been validated across the codebase. The grep results confirm that the variable is referenced as intended in both the application code (src/App.tsx) and the Tailwind configuration (tailwind.config.js), ensuring that arrow styling is consistently applied across all themes. No further changes are required.
- CSS usage verified in
src/index.css(lines 31, 55, 79, and 103)- Application reference found in
src/App.tsx- Tailwind configuration updated in
tailwind.config.jstailwind.config.js (3)
35-35: Validate thatarrow-coloris utilized across themes.
It appears you've introduced a newarrow-colorvariable; ensure it has consistent values defined in your CSS (e.g..light,.dark, etc.) so that it remains themable.
47-47: Solid addition for orange chip color.
This addition cleanly aligns with your existing palette. Looks good.
146-148: Good approach for animating height.
By specifying'height': 'height', Tailwind can now transition the height property, which is handy for dynamic layouts.src/app/components/fulltime/FullTime.tsx (7)
175-178: Clear separation ofdirectYcontent.
Displaying the ‘예술인 (yesul)’ times separately with a green chip is well structured and follows your existing pattern.
257-258:isShuttleIproperty is a clear flag for shuttle routes.
This addition nicely extends theOrganizedTimetablesobject without complicating the existing structure.
273-276: SeparatingisShuttleIlogic is straightforward.
MarkingisShuttleIwhenschedule.type === 'R'is an easy-to-read solution for highlighting shuttle routes.
288-290: EnsuringdirectYincrements the chip count.
This change is consistent with how other list types are handled. Good consistency.
296-296: No functional change.
This blank line addition is harmless; no further remarks.
303-304: Conditionally updating maximum chip count.
This logic correctly keepscountChipin sync with the highest count.
312-312: Conditional rendering for empty schedules.
Skipping theTimeBoxwhen all times are empty helps maintain a cleaner UI.src/App.tsx (10)
5-5: Transition import is appropriate for animations.
Bringing inreact-transition-groupis a good move for more controlled UI transitions.
10-10: Arrow icon import.
This import is straightforward and matches your usage of inline SVG React components.
14-14: RouteMap import for new UI feature.
This is a clear addition that integrates your new route mapping component.
19-19:Refreshingimport aligns with your pull-to-refresh functionality.
No concerns here. Nicely keeps related logic in a separate component.
121-123: StylingRouteIndexCardViewviastatusprop is a neat technique.
Leveraging styled-components for transitions ensures a clean approach to dynamic height changes.
130-134: Confirm inverted show/hide states forRouteIndexContainer.
Currently, the container is visible inexitedand hidden inentered, which is inverted from typical transitions. If that’s intentional, no issue; otherwise, consider reversing it.
135-138:RouteToggleImagetransition logic looks good.
Rotating the arrow based onenteredvs.exitedis intuitive and well-implemented.
162-162: Ensuringz-10keepsOptionWrapperin the foreground.
The style addition helps content remain on top of underlying elements.
199-199: New staterouteCardClicksupporting expanded/collapsed views.
This well-named state cleanly toggles the route card UI.
448-479: Smooth transitions with<Transition>block.
The setup forenteredandexitedstates is well-structured. Verify the final user experience to ensure it aligns with your design intent, especially regarding toggles and any potential reflow..pnp.cjs (8)
31-37: Addition of @types/react-transition-group Dependency
A new entry for@types/react-transition-group(version4.4.11) has been added. This should provide the necessary TypeScript typings for thereact-transition-grouplibrary. Please verify that the version aligns with other type definitions used in your project.
63-69: Inclusion of react-transition-group Dependency
The direct dependency forreact-transition-group(version4.4.5) is introduced here. Confirm that this version is consistent with the usage in your UI components and that there are no conflicts with transitive dependencies.
2729-2741: New Babel Runtime Package Block Addition
A new package block for npm package@babel/runtime(version7.26.0) is added, along with its dependency onregenerator-runtime(version0.14.1). Please ensure this block is properly nested and consistent with your intended Yarn Plug'n'Play configuration.
4121-4134: Addition of @types/react-transition-group Package Block
A dedicated package block for@types/react-transition-group(version4.4.11) is now provided, including its package location and dependency on@types/react(version18.3.1). Verify that this block is necessary and not redundant with similar entries elsewhere in the configuration.
5793-5809: New 'dom-helpers' Dependency Block
A package block fordom-helpers(version5.2.1) has been introduced with its dependencies on@babel/runtimeandcsstype. Ensure that these settings support the DOM manipulation utilities used in your UI components.
7229-7235: Type Definitions Group Consistency Check
The dependency for@types/react-transition-group(version4.4.11) appears within this type definitions group alongside other React-related types. Please verify that its inclusion here is intentional and does not create redundancy with similar entries in other sections.
7261-7267: Verify Consistency for react-transition-group in Dependency Groups
This section reiterates the dependency forreact-transition-group(version4.4.5) within the runtime dependency array. Confirm that having this entry in multiple groups is deliberate and that no version conflicts arise from its duplicated appearance.
9544-9577: Complex React-Transition-Group Package Block Addition
A comprehensive package block forreact-transition-group(version4.4.5) has been added, including both a standard and a virtual entry. The virtual block introduces multiple dependencies, peer dependencies, and uses aSOFTlink type. Please ensure that this complex configuration aligns with your overall dependency resolution strategy and that the virtual dependency behaves as expected.src/app/components/routemap/RouteMap.tsx (4)
1-5: Well-structured imports and modern React usage.
Imports and hooks usage (useCallback, useRef, etc.) look well-structured and consistent with current React conventions.
59-61: Confirm the status strings for transitions.
The condition checksprops.status === 'entered' || props.status === 'exit'to toggle opacity. Ensure that these match the transition states provided by your animation or React Transition Group. If additional states (like 'exiting') are used elsewhere, this might unintentionally hide or show the component.
214-233: Verify safety of DOM measurements.
responsiveLinesdirectly accesses offset properties. If the render timing changes (e.g., due to asynchronous events),offset*properties can be zero or undefined. Verify that each referenced DOM node exists and has stable measurements at the time of calculation.
254-270: No residual double-append issue detected.
Thanks for addressing the past concern about appending multiple circles. The conditional logic properly determines the correct index, and there’s no second unconditional append. It appears safe from duplication now.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/app/components/shuttle/Shuttle.tsx (1)
424-429:⚠️ Potential issueAdd error handling for empty filtered bus data
The current implementation doesn't check if filtered data is empty before accessing the first element, which could lead to setting undefined values in the context.
useEffect(() => { if(timetable.data !== undefined){ const filtered = timetable.data.filter((val) => isAfterCurrentTime(val)) - setTimetable(filtered[0]) + if (filtered.length > 0) { + setTimetable(filtered[0]) + } } }, [currentTime, setTimetable, timetable.data])
🧹 Nitpick comments (14)
src/app/data/routemap/circleAnimate.ts (1)
1-7: New type introduced for route animationThe
CircleAnimatetype is well-structured for managing DOM references and styling for animated route elements. This type will help maintain type safety when implementing the route map animations.Two minor style suggestions:
- Consider removing trailing commas at the end of property definitions for consistency
- Use consistent semicolon usage (either add semicolons at the end of all statements or remove them all)
import React from 'react'; export type CircleAnimate = { - ref: React.MutableRefObject<Array<HTMLDivElement>>, - index: number, - chipColor: string, + ref: React.MutableRefObject<Array<HTMLDivElement>> + index: number + chipColor: string }package.json (1)
44-44: Added TypeScript type definitions for react-transition-group.Adding the type definitions ensures proper TypeScript integration with the react-transition-group library. Consider specifying a more precise version (like "^4.4.5") to match exactly with the runtime dependency for better consistency.
src/app/components/lang/lang.ko.json (1)
24-24: Added extra whitespace in the "residence" translation.There's an extra space at the end of the "residence" translation value. While this won't affect functionality, it's better to maintain consistent formatting.
- "residence": "기숙사 (셔틀콕행)", + "residence": "기숙사 (셔틀콕행)",.pnp.cjs (4)
31-37: Entry for @types/react-transition-group Added
The addition of["@types/react-transition-group", "npm:4.4.11"]is correctly incorporated into the dependency array. Please verify that this version and its dependency on@types/react(version"npm:18.3.1") align with your project’s compatibility requirements.
63-69: New Dependency: react-transition-group Added
The dependency entry forreact-transition-groupwith version"virtual:a0dcacbc896eccf189caadb21dc337a943d120637d608a889ce480f30d60747b2e369188c5cb540e65c153429bc715bc64aac1ec96b7e01c4ef9b6515ffe1d10#npm:4.4.5"is now present. Ensure that this version meets the peer requirements for the components using animated transitions.
4121-4134: Detailed Block for @types/react-transition-group
A new block with detailed metadata (including package location and its dependency on@types/react) has been added for@types/react-transition-group. Confirm that this block’s information (especially the cache path and linkType) matches your build and runtime expectations.
5793-5809: Addition of dom-helpers Dependency Block
A new dependency block fordom-helpers(version5.2.1) has been added with its specific package location and dependencies (including@babel/runtimeandcsstype). Please ensure that this addition does not conflict with any existing usage and that the referenced versions are as intended.src/app/components/fulltime/FullTime.tsx (1)
179-192: Simplify membership check using.includes()
Instead of mapping solely to toggle a boolean, consider using.includes(time)for readability and minor performance gains.- let isExist = false - props.directY.map((ytime) => { - time === ytime ? (isExist = true) : null - }) - return ( - isExist ? null : - <React.Fragment key={idx}> - <span>{time} </span> - </React.Fragment> - ) + const isExist = props.directY.includes(time); + return isExist + ? null + : ( + <React.Fragment key={idx}> + <span>{time} </span> + </React.Fragment> + );src/App.tsx (2)
121-124: Handle all transition states
The styled component only handles'entered'versus others. If smoother transitions or partial animation states are needed, consider styling'entering'and'exiting'too.
448-478: Granular transitions
You’re handling'entered'and'exited'states. For smoother animations, consider tailoring'entering'and'exiting'states if needed.src/app/components/routemap/RouteMap.tsx (4)
67-71: Avoid reusingkeyprop
React reserveskeyfor internal usage, so it won’t behave as a normal prop. Consider renaming it, e.g.,stationIndex.
76-101: Logical state separation
Individually storing references and arrays for each route category works but could become unwieldy for more routes. Keep an eye on maintainability.
175-212: Shared line logic
Refactoring repeated line creation into a function with route definitions could foster DRY principles and easier maintenance.
306-341: Consolidate highlight logic
Expanding route coverage can make these conditions harder to manage. A data-driven approach may reduce complexity.
📜 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 (16)
.pnp.cjs(8 hunks)package.json(2 hunks)src/App.tsx(5 hunks)src/app/components/fulltime/FullTime.tsx(6 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/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/shuttle.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] 294-294: 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 (50)
src/app/data/index.ts (1)
5-5: Export type is now properly implementedThe export for
CircleAnimatetype now correctly references an existing module at./routemap/circleAnimate. The file has been created with the proper type definition, resolving the previous build pipeline failure.src/app/network/shuttle.ts (1)
16-16:❓ Verification inconclusive
API endpoint version upgrade
The change from
/timetable/${season}/${week}/${location}to/v2/timetable/${season}/${week}/${location}indicates a version update in the API. This is a good practice for API evolution.
🏁 Script executed:
#!/bin/bash # Check if the new v2 endpoint is consistently used throughout the codebase rg -g "*.{ts,tsx}" "/timetable/\$\{season\}/\$\{week\}/\$\{location\}" --iglob "!src/app/network/shuttle.ts"Length of output: 107
API Endpoint Version Upgrade & Consistency Check
The updated endpoint in
src/app/network/shuttle.tsnow uses`/v2/timetable/${season}/${week}/${location}`which aligns with our API evolution strategy. An automated search for the legacy
/timetable/${season}/${week}/${location}outside this file produced no output, suggesting that the update is localized. However, the absence of output means the inference isn’t definitive. Please manually verify that the legacy endpoint isn’t inadvertently used elsewhere in the codebase.
- Confirm that
/v2/timetable/${season}/${week}/${location}is the only endpoint used.- Double-check other parts of the codebase to ensure no references to the non-versioned endpoint remain.
src/app/data/shuttle/organizedTimetables.ts (1)
6-7: Ensure initialization of new propertiesThe
OrganizedTimetablestype has been extended withjungangandisShuttleIproperties, which is good for supporting the route map functionality. However, as noted in previous reviews, ensure these properties are properly initialized in all places whereOrganizedTimetablesobjects are created.#!/bin/bash # Find all locations where OrganizedTimetables objects are created # to verify proper initialization of new properties rg -g "*.{ts,tsx}" -l "OrganizedTimetables" | xargs rg -g "*.{ts,tsx}" "{" --context 3package.json (1)
34-34: Added react-transition-group dependency for animation support.This addition introduces the react-transition-group library, which will enable animated transitions in the application. This aligns with the PR's purpose of adding a route map, as transitions can enhance the user experience when navigating between different views or states.
src/main.tsx (3)
12-12: Added TimeTableContextProvider import.This import introduces a new context for managing timetable data, which will likely be used by the route map component to display schedule information.
44-44: Updated z-index for Snowfall component to 2.The z-index change to 2 ensures proper visual layering with other components in the route map, following the z-index hierarchy discussed in previous reviews (Line: 0, Circle: 1, Snowfall: 2).
50-52:❓ Verification inconclusive
Wrapped App with TimeTableContextProvider.
Good implementation of the context provider. This ensures that timetable data is available throughout the application, which is especially important for the newly added route map functionality.
It would be helpful to verify that components using this context implement proper error handling for cases when timetable data might be unavailable or loading.
🏁 Script executed:
#!/bin/bash # Search for components that use the TimeTableContext echo "Searching for components that use the TimeTableContext..." rg -A 3 "useTimeTableContext|TimeTableContext" --type-add "tsx:*.tsx" --type tsxLength of output: 3296
Action Required: Verify Error Handling for Unavailable/Loading Timetable Data
The integration of the
TimeTableContextProviderlooks solid, and the context is correctly provided across the application. TheuseTimeTableContexthook already includes an error check to ensure the provider exists. However, based on our search:
- Components Usage: Both the
RouteMapandShuttlecomponents useuseTimeTableContextdirectly (e.g., accessingtimetableandsetTimetable) without additional safeguards.- Loading/Unavailable Data: While the hook ensures that a missing provider leads to an immediate error, it does not handle situations where
timetablemight still benull(e.g., during data loading). This might lead to unexpected behavior or runtime errors if the consuming components do not account for anulltimetable.Please verify if additional error or loading state handling is intended or required within these components. If a fallback UI or error boundary is appropriate for handling the
nullstate oftimetable, consider adding the necessary checks.src/app/components/lang/lang.ko.json (2)
12-14: Added Korean translations for location names.The addition of "yesul" (예술인) and "jung" (중앙역) translations supports the route map feature by providing proper Korean labels for these locations.
25-25: Added "dorm" translation.Adding the "dorm" translation (기숙사) provides consistency with the English localization and supports the route map feature.
src/app/components/lang/lang.en.json (3)
12-14: Added English translations for location names.The addition of "yesul" (Yesulin), "jung" (Jungang), and proper spacing ensures consistent internationalization support for the route map feature.
25-25: Added "dorm" translation.Good addition of the "dorm" translation, maintaining consistency with the Korean localization file.
1-126:✅ Verification successful
Verify translations are used correctly in the RouteMap component.
The newly added translations should be properly utilized in the route map implementation. Let's verify this to ensure a consistent user experience across languages.
🏁 Script executed:
#!/bin/bash # Search for the RouteMap component implementation and how it uses translations echo "Checking RouteMap component implementation..." rg -A 5 "RouteMap" --type-add "tsx:*.tsx" --type tsx echo "Checking translation usage in RouteMap..." rg -A 3 "useTranslation|t\(['\"]yesul['\"]|t\(['\"]jung['\"]|t\(['\"]dorm['\"]" --type-add "tsx:*.tsx" --type tsxLength of output: 9383
Translation keys in RouteMap have been verified and are correctly implemented.
The inspection confirms that the RouteMap component is utilizing the appropriate translation keys (e.g.,
yesul,jung,dorm,dest_shuttle_o, anddest_subway) as defined insrc/app/components/lang/lang.en.json. No discrepancies were found between the defined translations and their usage within the component, ensuring a consistent user experience across languages.src/app/context/TimeTableContext.tsx (1)
1-27: Well-structured React context implementationThis new TimeTable context implementation follows React best practices with a clear interface definition, proper typing with TypeScript, and a useful custom hook that includes error handling. The pattern of providing a custom hook with context validation is particularly good for developer experience.
src/index.css (4)
31-31: Theme color addition for arrow elementsThe new CSS custom property
--color-arrow-coloris correctly added to the light theme, providing consistent styling capabilities for arrow elements.
55-55: Theme color addition for arrow elementsThe new CSS custom property
--color-arrow-coloris correctly added to the dark theme, providing consistent styling capabilities for arrow elements.
79-79: Theme color addition for arrow elementsThe new CSS custom property
--color-arrow-coloris correctly added to the Christmas theme, providing consistent styling capabilities for arrow elements.
103-103: Theme color addition for arrow elementsThe new CSS custom property
--color-arrow-coloris correctly added to the Spring theme, providing consistent styling capabilities for arrow elements.src/app/components/shuttle/Shuttle.tsx (3)
13-13: Import of new TimeTable context hookThe import of the newly created TimeTable context hook is properly added, enabling access to the timetable state management functionality.
353-355: Use of TimeTable contextGood implementation of extracting the
setTimetablefunction from the TimeTable context for later use in the component.
339-341: Extended chip styling for additional bus typesThe condition is expanded to handle 'R' and 'NA' bus types with the new orange chip, improving visual differentiation between bus types.
tailwind.config.js (4)
14-15: New responsive breakpoints addedTwo new screen breakpoints (
wideandmwide) have been added to enhance responsive design capabilities. These breakpoints are well-defined with appropriate minimum widths.
35-35: New arrow color variableThe
arrow-coloris properly added to reference the CSS custom property--color-arrow-color, ensuring consistent usage across the application through Tailwind's color system.
47-47: New chip color for bus typesThe
chip-orangecolor is properly added with the hex value#FFAB40, which will be used for styling 'R' and 'NA' bus types in the ColoredChip component.
146-148: New transition property for height animationsThe transition property for height is correctly added, enabling smooth height animations in components using Tailwind's transition utilities.
.pnp.cjs (4)
2729-2741: Reorganization of @babel/runtime Block
The restructuring of the block containing@babel/runtimeand its dependency onregenerator-runtimeappears intended to group metadata such as the package location and dependencies. Please verify that the new block structure is correctly resolved by the Plug'n'Play system and that the versions remain consistent.
7229-7235: Consistency Check: @types/react-transition-group Entry
This segment reaffirms the inclusion of["@types/react-transition-group", "npm:4.4.11"]. Double-check that if multiple declarations appear in different parts of the file, they are intentional and won’t lead to conflicts in dependency resolution.
7261-7267: Reaffirmation of react-transition-group Dependency
The entry forreact-transition-groupwith version4.4.5is repeated in this section. Please confirm that these multiple records are required by different parts of the dependency tree and that they don’t cause accidental duplication or resolution issues.
9544-9577: Detailed Configuration for react-transition-group with Virtual Dependencies
A comprehensive block forreact-transition-groupnow includes both a standard entry and a virtual entry (notably with alinkTypeof"SOFT"for one variant). Verify that the configuration—including the extensive dependency tree and packagePeers—is exactly as intended and does not conflict with the previously declared instances.src/app/components/fulltime/FullTime.tsx (6)
175-178: No issues found
The additionalTimeBoxBodyGridfordirectYintegrates well with the existing structure.
257-276: Verify correct handling of multiple 'R' schedules
Currently, if multiple schedules in the same hour have type'R',isShuttleIis simply set once. Confirm that this single boolean covers your intended behavior for multiple'R'entries in the same hour.
288-290: Consistent route count increment
Incrementingsingle.countfor non-emptydirectYaligns with the established pattern for other route types.
296-296: No functional changes
This blank line adds no new logic.
312-313: Conditionally skipping empty schedules
This short-circuit logic effectively prevents rendering an emptyTimeBox.
319-319: New prop for schedule styling
Passing theisShuttleIprop enables custom styling and logic for 'R' schedules.src/App.tsx (4)
5-19: New imports
All necessary imports look correct. Ensurereact-transition-groupis installed at a suitable version.
130-134: Verify reversed logic
Whenstatus === 'exited', the container is fully opaque, whilestatus === 'entered'sets it tohidden. Confirm that this inverted approach matches your UX/design intent.
135-138: Transition-based rotation
Rotating the arrow based on the transition state is a neat way to visualize expansion/collapse.
199-199: No concerns
Using state forrouteCardClickis straightforward and appropriate for toggling the transition.src/app/components/routemap/RouteMap.tsx (12)
1-8: No issues found
Imports appear correct, ensuring you have all the dependencies required.
9-36: Consistent color-coded components
Each styled circle is well-defined for its route type, enhancing visual clarity.
37-66: Well-structured layout
Using grid-based containers for routes and stations is a clear, modular approach.
72-75: Clear component signature
ExportingRouteMapwith explicit prop types clarifies usage.
214-233: Dynamic line positioning
Calculating offsets and distances for each segment is valid. Ensure complex layouts remain accurate on various screen sizes.
235-241: Encapsulated line updates
Centralizing the line-update steps clarifies your approach and simplifies calling them from useEffects.
242-252: Localized text updates
Dynamically updating special stop labels via refs is effective. Just ensure indices remain correct after expansions.
254-280: Double-check appended circle color
The ping circle defaults tobg-chip-orangeifi === 5or(i === 4 && chipColor === 'bg-chip-blue'). Verify this meets your route color logic.
281-291: Centralized mapping
Returning references and color codes based on schedule types is concise.
293-295: Language synchronization
CallingfetchLanguages()on language changes ensures translations remain up to date.
297-305: Initialize route visual elements
InvokingdirectInput()anddirectLineInput()once is a clean setup step for the layout.
343-392: Key-based rendering
Using.map()to render circles and lines is fine. Just ensure each item has its own unique key. Looks solid overall.
| const directInput = useCallback(() => { | ||
| const arrdir = [] | ||
| const arrcyc = [] | ||
| const arryes = [] | ||
| const arrjun = [] | ||
| for(let i = 0; i < 5; i++){ | ||
| if(i === 4){ | ||
| arrdir.push(<RouteStations key={i} ref={d => d!= null?refdir.current[i] = d:null}><EndCircle/></RouteStations>) | ||
| continue | ||
| } | ||
| arrdir.push(<RouteStations key={i} ref={d => d!= null?refdir.current[i] = d:null}><DirectCircle/></RouteStations>) | ||
| } | ||
| for(let i = 0; i < 6; i++){ | ||
| if(i >= 2 && i <= 4){ | ||
| if(i === 2){ | ||
| arrcyc.push( | ||
| <div key={i} className='col-span-2 grid grid-cols-3 w-[75%] place-items-center'> | ||
| <RouteStations key={i} ref={d => d!= null?refcyc.current[i] = d:null}><CycleCircle/></RouteStations> | ||
| <RouteStations key={i+1} ref={d => d!= null?refcyc.current[i+1] = d:null}> | ||
| <CycleCircle className='grid grid-rows-2 relative'> | ||
| <SpecialStopsText key={0} ref={d => lang.current[0] = d} lang={i18n.language} className='text-chip-red'> | ||
| {t('yesul')} | ||
| </SpecialStopsText> | ||
| </CycleCircle> | ||
| </RouteStations> | ||
| <RouteStations key={i+2} ref={d => d!= null?refcyc.current[i+2] = d:null}><CycleCircle/></RouteStations> | ||
| </div> | ||
| ) | ||
| arryes.push( | ||
| <div key={i} className='col-span-2 grid grid-cols-3 w-[75%] place-items-center'> | ||
| <RouteStations key={i} ref={d => d!= null?refyes.current[i] = d:null} title='skip'><YesulinCircle className='opacity-0'/></RouteStations> | ||
| <RouteStations key={i+1} ref={d => d!= null?refyes.current[i+1] = d:null}> | ||
| <YesulinCircle className='grid grid-rows-2 relative'> | ||
| <SpecialStopsText key={1} ref={d => lang.current[1] = d} lang={i18n.language} className='text-chip-green'> | ||
| {t('yesul')} | ||
| </SpecialStopsText> | ||
| </YesulinCircle> | ||
| </RouteStations> | ||
| <RouteStations key={i+2} ref={d => d!= null?refyes.current[i+2] = d:null}><YesulinCircle/></RouteStations> | ||
| </div> | ||
| ) | ||
| arrjun.push( | ||
| <div key={i} className='col-span-2 grid grid-cols-3 w-[75%] place-items-center'> | ||
| <RouteStations key={i} ref={d => d!= null?refjun.current[i] = d:null}><JungangCircle/></RouteStations> | ||
| <RouteStations key={i+1} ref={d => d!= null?refjun.current[i+1] = d:null}> | ||
| <JungangCircle className='grid grid-rows-2 relative'> | ||
| <SpecialStopsText key={2} ref={d => lang.current[2] = d} lang={i18n.language} className='text-chip-purple'> | ||
| {t('jung')} | ||
| </SpecialStopsText> | ||
| </JungangCircle> | ||
| </RouteStations> | ||
| <RouteStations key={i+2} ref={d => d!= null?refjun.current[i+2] = d:null}><JungangCircle/></RouteStations> | ||
| </div> | ||
| ) | ||
| } | ||
| continue | ||
| } | ||
| if(i === 5){ | ||
| arrcyc.push(<RouteStations key={i} ref={d => d!= null?refcyc.current[i] = d:null}><EndCircle/></RouteStations>) | ||
| arryes.push(<RouteStations key={i} ref={d => d!= null?refyes.current[i] = d:null}><EndCircle/></RouteStations>) | ||
| arrjun.push(<RouteStations key={i} ref={d => d!= null?refjun.current[i] = d:null}><EndCircle/></RouteStations>) | ||
| continue | ||
| } | ||
| arrcyc.push(<RouteStations key={i} ref={d => d!= null?refcyc.current[i] = d:null}><CycleCircle/></RouteStations>) | ||
| arryes.push(<RouteStations key={i} ref={d => d!= null?refyes.current[i] = d:null}><YesulinCircle/></RouteStations>) | ||
| arrjun.push(<RouteStations key={i} ref={d => d!= null?refjun.current[i] = d:null}><JungangCircle/></RouteStations>) | ||
| } | ||
| setDirect(arrdir) | ||
| setCycle(arrcyc) | ||
| setYesulin(arryes) | ||
| setJungang(arrjun) | ||
| },[i18n.language, t]) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider a data-driven approach
Numerous conditional branches suggest a more unified strategy might reduce repetition and make updates simpler.
There was a problem hiding this comment.
Pull Request Overview
This pull request introduces a new interactive route map with dynamic animated transitions and enhances shuttle schedule display logic by integrating the TimeTableContext into various components. Key changes include:
- Adding the TimeTableContext and its provider to manage timetable state.
- Implementing the RouteMap component with animated circle and line drawing logic.
- Updating shuttle schedule filtering, API endpoints, and UI adjustments for full-time schedule displays.
Reviewed Changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/app/context/TimeTableContext.tsx | Introduces and provides timetable context for state management. |
| src/app/components/routemap/RouteMap.tsx | Implements the interactive route map with animation and responsive lines. |
| src/app/data/routemap/circleAnimate.ts | Exports the CircleAnimate type for use in animated circle logic. |
| src/App.tsx | Integrates RouteMap via Transition and adjusts UI component layouts. |
| src/main.tsx | Wraps App with the new TimeTableContextProvider and adjusts z-index. |
| src/app/components/shuttle/Shuttle.tsx | Enhances shuttle schedule filtering using the TimeTableContext data. |
| src/app/data/shuttle/organizedTimetables.ts | Adds a new boolean flag (isShuttleI) to the organized timetables type. |
| src/app/network/shuttle.ts | Updates the API endpoint version to v2 for shuttle schedules. |
| src/app/components/fulltime/FullTime.tsx | Updates timetable UI layout and conditionals based on new shuttle flags. |
| src/app/data/index.ts | Exports updated type definitions, including CircleAnimate. |
Comments suppressed due to low confidence (2)
src/app/components/routemap/RouteMap.tsx:255
- Accessing props.ref.current[i] without checking if it exists may lead to runtime errors. Add a check to ensure that props.ref.current[i] is defined before accessing its childNodes.
for(let i = props.index; i <= props.index+1 && props.ref.current[i].childNodes.length <= 1; i++){
src/app/components/shuttle/Shuttle.tsx:425
- timetable is initially null in the TimeTableContext, so accessing timetable.data without first confirming that timetable is not null could cause errors. Consider adding a null check before accessing timetable.data.
if(timetable.data !== undefined){
| @@ -292,20 +302,21 @@ const FullTime = () => { | |||
| if (maxCount > countChip) { | |||
| setCountChip(maxCount) | |||
| } | |||
|
|
|||
There was a problem hiding this comment.
Is the file properly formatted? There are empty spaces in empty lines
There was a problem hiding this comment.
From L98~L106, check if those attributes are used after the update.
There was a problem hiding this comment.
Could you elaborate a little more on this review?
There was a problem hiding this comment.
const Button = styled(CardView)`
${tw`
flex will-change-transform overflow-hidden cursor-default
border-none px-2 py-6 hm:py-4 hm:text-sm hm:leading-4 text-theme-text
`}
&.active {
${tw`
bg-button-active text-black drop-shadow-none shadow-inner transition-all ease-out duration-700
`}
}
&#shuttlecoke_i {
${tw`shuttlei:flex-col shuttlei:gap-x-0 gap-x-1 items-center justify-center`}
}
`Check if selector .active and shuttlecoke_i is used at the moment.
src/app/context/TimeTableContext.tsx
Outdated
There was a problem hiding this comment.
As the latest copilot review mentioned:
src/app/components/shuttle/Shuttle.tsx:425
- timetable is initially null in the TimeTableContext, so accessing timetable.data without first confirming that timetable is not null could cause errors. Consider adding a null check before accessing timetable.data.
Is there a reason why timetable and setTimetable value can be null?
|
Check out your Lighthouse Report: https://lighthouse.hybus.app/app/projects/bushanyang-production/dashboard |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
src/app/components/shuttle/Shuttle.tsx (1)
424-429:⚠️ Potential issueAdd error handling for empty filtered bus data.
The current implementation could lead to setting undefined data in the context if no buses meet the filtering criteria.
Apply this fix to prevent potential errors:
useEffect(() => { if(timetable.data !== undefined){ const filtered = timetable.data.filter((val) => isAfterCurrentTime(val)) - setTimetable(filtered[0]) + if (filtered.length > 0) { + setTimetable(filtered[0]) + } } }, [currentTime, setTimetable, timetable.data])src/app/components/fulltime/FullTime.tsx (2)
183-192:⚠️ Potential issueFix missing key in React fragment.
While the logic to prevent duplicate time entries is good, there's an issue with the React key implementation.
React requires a unique key for elements in an array to optimize rendering. Apply this fix:
-<React.Fragment key={idx}> +<React.Fragment key={`direct-${time}-${idx}`}> <span>{time} </span> </React.Fragment>
293-294:⚠️ Potential issueMissing key property for element in iterable.
The return statement inside the
timetableFiltered.forEachloop is creating elements in an iterable without a key property.- return <></> + // This return statement isn't necessary since forEach doesn't use the return value + // If you need it, add a key: + // return <React.Fragment key={hour}><></></React.Fragment>🧰 Tools
🪛 Biome (1.9.4)
[error] 294-294: 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)
src/app/components/routemap/RouteMap.tsx (1)
82-115: 🛠️ Refactor suggestionReduce deeply nested if-else logic.
The
isPrevStopfunction is complex. A data-driven table or dictionary-based approach would make it easier to add or change conditions without nesting repeatedly.Consider this structure:
- if (props.tab === 'shuttlecoke_o') { - if (index === 0) return false - return true - } else if (props.tab === 'shuttlecoke_i') { - ... - } + const rules: Record<string, (line: string, index: number) => boolean> = { + shuttlecoke_o: (line, index) => index !== 0, + shuttlecoke_i: (line, index) => line === 'direct' ? index >= 3 : index >= 4, + ... + } + return rules[props.tab]?.(line, index) ?? true
🧹 Nitpick comments (9)
src/app/data/routemap/circleAnimate.ts (1)
1-7: New type definition for CircleAnimateThe
CircleAnimatetype definition looks good for managing circular animations in React components. It provides a structured way to reference DOM elements and their properties.A few considerations:
- The ref to an array of DOM elements could potentially cause memory leaks if elements are continuously added but never removed. Ensure proper cleanup in the component lifecycle.
- Consider using TypeScript's more strict comma conventions - trailing commas are used in some places but not others.
export type CircleAnimate = { - ref: React.MutableRefObject<Array<HTMLDivElement>>, - index: number, - chipColor: string, + ref: React.MutableRefObject<Array<HTMLDivElement>>; + index: number; + chipColor: string; }src/app/context/TimeTableContext.tsx (1)
1-27: Consider using non-nullable context for better type safety.The context implementation looks clean, but there's a potential issue with the initial null value. While the hook properly throws an error when the context is undefined, consumers of the timetable value would still need to handle null cases.
Consider providing a safer default value to avoid potential runtime errors:
-const TimeTableContext = React.createContext<TimeTableContextProps | null>(null) +// Use a default empty state to avoid null checks +const defaultTimeTableContext: TimeTableContextProps = { + timetable: null, + setTimetable: () => { + console.warn('TimeTable context used outside of provider') + } +} + +const TimeTableContext = React.createContext<TimeTableContextProps>(defaultTimeTableContext) export const useTimeTableContext = () => { - const context = React.useContext(TimeTableContext) - if (!context) throw Error('TimeTable provider not defined!') - return context + return React.useContext(TimeTableContext) }This feedback is based on a previous review comment about potential null reference issues that was mentioned in connection with
timetableandsetTimetable.src/index.css (4)
23-23: Improved Shadow Styling with Rem Units (Light Theme)
The shadow property for the light theme has been updated to use rem-based values instead of fixed pixel values. This change improves responsiveness and ensures that the shadow scales appropriately with the user’s font settings.
47-47: Updated Shadow Styling with Rem Units (Dark Theme)
The dark theme’s shadow property is now defined with rem units. This update aligns the shadow styling with modern responsive design practices.
71-71: Enhanced Shadow Effect (Christmas Theme)
The Christmas theme’s shadow value has been updated to use rem units. This change not only modernizes the code but also ensures better adaptability on devices with varying settings.
95-95: Updated Shadow Styling with Rem Units (Spring Theme)
For the Spring theme, the shadow property has been refined by using rem units, which enhances scalability and consistency across different display environments.src/App.tsx (3)
120-124: Handle additional transition states for smoother animations.Currently, the styled component only distinguishes between
'entered'and'exited'states. If you need smoother transitions, consider also handling'entering'and'exiting':-const RouteIndexCardView = styled(CardView)<{status: string}>` - ${(props) => props.status === 'entered' ? tw`h-[13.7rem] hm:h-[11.7rem]` : tw`h-14 hsm:h-16`} +const RouteIndexCardView = styled(CardView)<{status: string}>` + ${({ status }) => { + switch (status) { + case 'entering': + return tw`h-[13rem] hm:h-[11rem]` + case 'entered': + return tw`h-[13.7rem] hm:h-[11.7rem]` + case 'exiting': + return tw`h-14 hsm:h-16` + case 'exited': + default: + return tw`h-14 hsm:h-16` + } + }}
130-134: Verify visibility logic for partial states.The container is fully hidden on
'entered'and visible on'exited', ignoring'entering'and'exiting'. If you want a staged fade or visibility transition, account for those states as well.
448-478: Use all four transition states for a more refined animation sequence.When passing
statefrom<Transition>, you have'entering','entered','exiting', and'exited'. Currently, only'exited'and'entered'are visually handled in the styled components.
📜 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 (16)
.pnp.cjs(8 hunks)package.json(2 hunks)src/App.tsx(5 hunks)src/app/components/fulltime/FullTime.tsx(6 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/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/shuttle.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/index.css
[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] 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)
src/app/components/fulltime/FullTime.tsx
[error] 294-294: 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 (40)
src/app/data/index.ts (1)
5-5: Export added for CircleAnimate typeGood to see this file has been created and is now properly exported. This should fix the previous build pipeline error that was reported in the earlier review.
src/app/network/shuttle.ts (1)
16-16:✅ Verification successful
API endpoint version updated to v2
The endpoint has been updated to use a versioned API path (
/v2/timetable/...). This is a good practice for API versioning, but ensure that:
- The backend server correctly handles this new v2 endpoint
- Any documentation is updated to reflect this change
- The v2 API behavior is properly tested with this client
🏁 Script executed:
#!/bin/bash # Check if there are any references to the old API endpoint format rg -g "*.{ts,tsx}" "/timetable/[^v]" --no-filenameLength of output: 102
Endpoint Version Updated Successfully
- The search confirms that the code now uses the updated v2 endpoint
/v2/timetable/${season}/${week}/${location}, with no references to the old API version.- Please ensure that the backend service is updated to handle this v2 endpoint, the relevant documentation reflects these changes, and tests are in place to verify the updated API behavior.
src/app/data/shuttle/organizedTimetables.ts (1)
6-7: New properties added to OrganizedTimetablesThe
jungangarray andisShuttleIboolean properties have been added to theOrganizedTimetablestype. While the type definition looks good, make sure these properties are properly initialized in all places whereOrganizedTimetablesobjects are created.From a previous review, it appears these properties might not be initialized in some places like
src/app/components/fulltime/FullTime.tsx. Please ensure all locations where this type is used initialize these properties with appropriate default values to avoid runtime errors.#!/bin/bash # Find all places where OrganizedTimetables objects are created rg -g "*.{ts,tsx}" ":\s*OrganizedTimetables\s*=\s*\{" -A 10 --no-filenamepackage.json (2)
34-34: Nice addition of React Transition Group!Adding this library is a good choice for implementing animated transitions, which will enhance the user experience with smooth effects when components mount or unmount.
44-44: Good practice including type definitions.Including the TypeScript type definitions for react-transition-group ensures proper type checking and IDE autocompletion support.
src/app/components/lang/lang.en.json (2)
12-14: Good localization additions.These new translation keys for "yesul", "jung" are properly aligned with their respective values and will help with location-specific text in the UI.
25-25: Consistent localization pattern.Adding the "dorm" key maintains consistency with the existing localization structure, making the codebase more maintainable.
src/main.tsx (3)
12-12: Good addition of TimeTableContext.Importing the TimeTableContextProvider is a good step toward better state management for timetable data across components.
44-44: Proper z-index adjustment.The z-index value has been correctly set to 2 as agreed in previous discussions, ensuring proper layering of the cherry blossom effect above other components.
50-52: Good context provider implementation.Wrapping the App component with TimeTableContextProvider ensures the timetable data is accessible throughout the component tree, which is a good pattern for shared state management.
src/app/components/shuttle/Shuttle.tsx (3)
13-13: Good context integration.Importing the useTimeTableContext hook allows for proper integration with the newly created shared state management system.
339-341: Enhanced ColoredChip component.The ColoredChip component now correctly handles both 'R' and 'NA' chip types with the orange background color, improving visual consistency.
354-354: Good use of context API.Destructuring setTimetable from the context is a clean approach to accessing only what's needed from the context.
src/app/components/lang/lang.ko.json (3)
12-12: Well-structured language key addition.The new key
"yesul": "예술인"for the artist location matches the naming patterns used throughout the file and will support the new route map functionality.
14-14: Consistent translation for central station.The added
"jung": "중앙역"translation key maintains consistency with the existing"jungang": "중앙역"(line 27) and supports the route map functionality.
24-25: Dormitory translation key looks good.The
"dorm": "기숙사"key appropriately matches the existing"residence": "기숙사 (셔틀콕행)"but provides a shorter version without destination context.src/app/components/fulltime/FullTime.tsx (7)
103-105: Good extension of TimeBoxInner height calculation.Adding support for 4 chips is appropriate given the addition of a new directY attribute to the component.
175-178: Proper implementation of the new directY (Yesulin) route display.The implementation of the green chip and route information display follows the same pattern as the existing routes.
180-180: Good use of conditional styling for shuttle type.The conditional class assignment using the new
isShuttleIproperty correctly differentiates between shuttle types with appropriate colors.
257-257: Good addition of isShuttleI property.The isShuttleI property allows for proper differentiation of shuttle types in the UI.
274-276: Clear implementation of the logic to set isShuttleI flag.This logic correctly identifies and flags routes of type 'R' as shuttle interior routes.
288-290: Proper count increment for directY entries.The implementation correctly updates the count for calculating maximum chips when directY entries exist.
312-312: Improved conditional rendering logic.The updated condition properly handles the new directY and jungang route types, ensuring components are only rendered when needed.
tailwind.config.js (4)
14-15: Good addition of wider screen breakpoints.The new
wideandmwidebreakpoints will help with responsive design for the route map component.
35-35: Appropriate use of CSS variable for arrow color.Using a CSS variable for arrow color will allow for theme-based styling changes.
47-47: Good addition of chip-orange color.The new chip-orange color (#FFAB40) complements the existing chip color palette and will be used for distinguishing the shuttle interior routes.
146-148: Good addition of height transition property.Adding a specific transition property for height will enable smooth animations for expanding/collapsing elements in the route map view.
src/index.css (5)
31-31: Addition of New Arrow Color Variable (Light Theme)
A new CSS variable--color-arrow-colorwith the value#abababhas been added to the light theme, which helps maintain consistent arrow styling across components.
55-55: Addition of New Arrow Color Variable (Dark Theme)
The dark theme now includes the--color-arrow-colorvariable set to#dbdbdb. This addition promotes visual consistency in the theme’s color palette.
79-79: Addition of New Arrow Color Variable (Christmas Theme)
A new arrow color variable--color-arrow-colorwith the value#000000is introduced for the Christmas theme. This helps provide a clearer visual contrast and aligns with the updated theme design.
103-103: Addition of New Arrow Color Variable (Spring Theme)
The Spring theme now defines--color-arrow-colorwith the value#bf6081. This new variable introduces a distinctive accent that complements the overall Spring styling.
24-29: Addressing Static Analysis Warnings fortheme()Function Usage
Static analysis tools have flagged the usage of thetheme()function (for example, on lines 24, 25, 26, 28, 29 in the light theme; lines 48, 49, 50, 52 in the dark theme; and line 76 in the Christmas theme). If these function calls are being correctly processed as part of your Tailwind CSS setup, consider updating your static analyzer configuration to recognize them. Otherwise, please verify that the usage oftheme()is valid or consider replacing them with standard CSS approaches.Also applies to: 48-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)
.pnp.cjs (8)
31-37: New Type Definitions Added
The addition of["@types/react-transition-group", "npm:4.4.11"]ensures TypeScript support forreact-transition-group, which aligns with the update in yourpackage.json. Verify that this new type is compatible with your installed React version.
63-69: New Package Dependency for React Transition Group
The added dependency["react-transition-group", "virtual:a0dcacbc896eccf189caadb21dc337a943d120637d608a889ce480f30d60747b2e369188c5cb540e65c153429bc715bc64aac1ec96b7e01c4ef9b6515ffe1d10#npm:4.4.5"],appears to be correctly integrated into the dependency array. Ensure that the integration of the virtual reference fits your resolution strategy.
2729-2742: Updated Babel Runtime Dependency Block
The modifications in this block clarify the dependency configuration for@babel/runtimeand its dependency onregenerator-runtime. The explicit definition of package location and dependencies (with link types maintained) should aid in consistent dependency resolution.
4121-4134: New Block for @types/react-transition-group
A dedicated dependency block for@types/react-transition-grouphas been introduced, complete with its package location and dependency on@types/react. This is well configured and reinforces type safety across your application.
5793-5809: Addition of dom-helpers Dependency Block
The inserted section fordom-helpers(version5.2.1) includes its package dependencies (such as@babel/runtimeandcsstype) and specifies a "HARD" link type. Please verify that this dependency is required and that its version is compatible with other UI-related utilities in your project.
7229-7235: Consistent Reiteration of Type Definitions
The reiteration of the["@types/react-transition-group", "npm:4.4.11"]entry in this section ensures consistency across the different dependency arrays in the RAW_RUNTIME_STATE. This redundancy appears intentional and helpful.
7261-7267: Parallel Addition of react-transition-group Dependency
This block reaffirms the inclusion ofreact-transition-groupusing its virtual package reference. The consistency with earlier entries in the file is commendable.
9544-9577: Comprehensive Dependency Block for react-transition-group
This final block introduces a detailed configuration forreact-transition-groupthat includes:
- An entry using the npm package with a
SOFTlink type- A virtual entry that further specifies extensive package dependencies and peer dependencies (e.g.,
@babel/runtime,@types/react,dom-helpers, etc.) with aHARDlink typePlease verify that this dual entry is intentional and necessary within your Yarn PnP setup to avoid potential conflicts or duplication. It might be useful to double-check that the differentiation between
SOFTandHARDlink types aligns with your dependency resolution strategy.
| if(props.tab === 'shuttlecoke_o'){ | ||
| if (index === 0) return false | ||
| else return true | ||
| } else if (props.tab === 'shuttlecoke_i'){ | ||
| if (line === 'direct') { | ||
| if (index < 3) return false | ||
| else return true | ||
| } else { | ||
| if (index < 4) return false | ||
| else return true | ||
| } | ||
| } else if (props.tab === 'subway'){ | ||
| if (line === 'yesulin') return false | ||
| else { | ||
| if (index < 2) return false | ||
| else return true | ||
| } | ||
| } else if (props.tab === 'yesulin'){ | ||
| if (line === 'direct' || line === 'jungang') return false | ||
| else { | ||
| if (index < 3) return false | ||
| else return true | ||
| } | ||
| } else if (props.tab === 'jungang'){ | ||
| if (line === 'jungang') { | ||
| if (index < 3) return false | ||
| else return true | ||
| } else return false | ||
| } else { | ||
| // domitory is a terminal | ||
| return true | ||
| } |
There was a problem hiding this comment.
This if-else statements can be simplified.
For example,
if (index === 0) return false
else return truecan be simplified as
return index !== 0Also, if it is simplified that way, I think there is no reason to use else-if statements since all if statements end with return statements. The code will never reach anyways if there are only if statements.
Describe your Pull Request
Please describe the behavior or changes of your code specifically.
Additional content
Any additional comments? 😁
Summary by CodeRabbit
New Features
Style