refactor: add options to hide schedule #1203
Conversation
|
Warning Rate limit exceeded@cpvalente has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 14 minutes and 26 seconds before requesting another review. How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughThe changes introduce a new CSS class modifier for layout management, enhance the Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Outside diff range, codebase verification and nitpick comments (1)
apps/client/src/features/viewers/studio/StudioClock.tsx (1)
Line range hint
64-117: Component logic and JSX structure review.The component logic has been significantly refactored to include conditional rendering based on the
hideRightparameter. This change is well-aligned with the PR objectives and enhances the component's interactivity by allowing users to toggle the schedule display based on their preferences.However, there are a few areas that could be improved for better maintainability and readability:
- Handling of Time Formatting: The logic for handling AM/PM in the clock display is somewhat convoluted and could be simplified. Consider using a dedicated utility function for this purpose to make the code cleaner and more maintainable.
- Conditional Rendering: The conditional rendering logic for displaying the
StudioClockSchedulecomponent is straightforward, but ensure that it is robust against all edge cases, especially when thehideRightparameter is toggled frequently.Overall, the changes are in line with the objectives, but refining these areas could enhance the code quality further.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
packages/utils/index.tsis excluded by none and included by none
Files selected for processing (6)
- apps/client/src/features/viewers/studio/StudioClock.scss (1 hunks)
- apps/client/src/features/viewers/studio/StudioClock.tsx (6 hunks)
- apps/client/src/features/viewers/studio/StudioClockSchedule.tsx (1 hunks)
- apps/client/src/features/viewers/studio/studioClock.options.ts (1 hunks)
- apps/client/src/features/viewers/studio/studioClock.utils.ts (2 hunks)
- packages/utils/src/date-utils/conversionUtils.ts (1 hunks)
Additional comments not posted (8)
apps/client/src/features/viewers/studio/studioClock.options.ts (1)
9-16: New option addition is well-implemented.The addition of the
hideRightoption is correctly implemented and follows the established pattern for configuration options. The default value offalseis appropriate, ensuring backward compatibility and user-driven customization.apps/client/src/features/viewers/studio/studioClock.utils.ts (1)
Line range hint
1-16: Remaining functiontrimRundownis correctly implemented.The function is well-documented and logically sound. It handles the trimming of event lists based on a selected ID and a limit, which is a typical use case in event management systems.
packages/utils/src/date-utils/conversionUtils.ts (1)
67-78: New functionsecondsInMillisis correctly implemented.The function is well-documented and efficiently handles the conversion of milliseconds to seconds, considering edge cases like falsy values. This utility function is a valuable addition to the module.
apps/client/src/features/viewers/studio/StudioClockSchedule.tsx (2)
1-8: Imports are well-organized and relevant.The imports in this file are well-organized and directly relevant to the functionality of the
StudioClockSchedulecomponent. This helps in maintaining clarity and ensuring that all dependencies are correctly managed.
10-15: InterfaceStudioClockSchedulePropsis well-defined.The interface
StudioClockSchedulePropsis clearly defined and includes all necessary properties (rundown,selectedId,nextId,onAir) that the component requires to function properly. This ensures strong typing and helps in maintaining the integrity of data passed to the component.apps/client/src/features/viewers/studio/StudioClock.scss (1)
45-48: CSS changes for.hide-rightclass are appropriate.The addition of the
.hide-rightclass to adjust the grid layout is well-implemented. It correctly sets thegrid-template-columnsandgrid-template-areasto focus solely on the clock area when the schedule is not needed. This change aligns with the PR objectives to allow users to disable the right section of the studio clock interface for a more streamlined view.Ensure to verify the visual impact in different screen sizes and configurations to confirm that the layout behaves as expected across all devices.
Consider adding screenshots or conducting a visual regression test to ensure that the changes appear as intended across all supported devices and resolutions.
apps/client/src/features/viewers/studio/StudioClock.tsx (2)
Line range hint
1-16: Imports are well-organized and relevant.The imports in this file are well-organized and directly relevant to the functionality of the
StudioClockcomponent. This helps in maintaining clarity and ensuring that all dependencies are correctly managed.
25-26: Changes toStudioClockPropsinterface are appropriate.The update of
selectedIdandnextIdproperties fromstring | nulltoMaybeStringin theStudioClockPropsinterface is a positive change. It enhances the flexibility in handling these properties and aligns with the use ofMaybeStringacross other components. This change should help in managing optional string values more effectively.
| export default function StudioClockSchedule(props: StudioClockScheduleProps) { | ||
| const { rundown, selectedId, nextId, onAir } = props; | ||
|
|
||
| const delayed = rundown.filter((event) => isOntimeEvent(event)) as OntimeEvent[]; | ||
| const trimmedRundown = trimRundown(delayed, selectedId, MAX_TITLES); | ||
|
|
||
| return ( | ||
| <div className='schedule-container'> | ||
| <div className={onAir ? 'onAir' : 'onAir onAir--idle'} data-testid={onAir ? 'on-air-enabled' : 'on-air-disabled'}> | ||
| ON AIR | ||
| </div> | ||
| <ul className='schedule'> | ||
| {trimmedRundown.map((event) => { | ||
| const start = formatTime(event.timeStart + (event?.delay ?? 0), { format12: 'h:mm a', format24: 'HH:mm' }); | ||
| const isSelected = event.id === selectedId; | ||
| const isNext = event.id === nextId; | ||
| const classes = `schedule__item schedule__item${isSelected ? '--now' : isNext ? '--next' : '--future'}`; | ||
| return ( | ||
| <li key={event.id} className={classes}> | ||
| <span className='event'> | ||
| <span className='event__colour' style={{ backgroundColor: `${event.colour}` }} /> | ||
| <SuperscriptTime time={start} /> | ||
| </span> | ||
| <span>{event.title}</span> | ||
| </li> | ||
| ); | ||
| })} | ||
| </ul> | ||
| </div> | ||
| ); | ||
| } |
There was a problem hiding this comment.
Component logic and JSX structure review.
The component logic is mostly clear and functional. However, there are a few areas that could be improved:
- Conditional Class Application: The way CSS classes are applied based on the
onAirprop could be simplified for better readability. Consider using a utility function to handle class names based on conditions. - Event Filtering: The filtering of events to check if they are
OntimeEventinstances is crucial. Ensure that this logic is robust and consider adding error handling if the data structure is not as expected. - TODO Comment: The TODO comment about fitting titles on the screen suggests that there might be UI issues with long titles. It would be beneficial to address this or at least track it in your project management tool.
Overall, the component does what it is supposed to do but consider refining these areas to enhance maintainability and readability.
cpvalente
force-pushed
the
studio-options
branch
from
88fb40d
to
6fe0482
Compare
Adds option to disable right section in studio clock so we only get the clock view in the center
As described in #1168