Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: timeline view #1131

Merged
merged 1 commit into from
Aug 4, 2024
Merged

feat: timeline view #1131

merged 1 commit into from
Aug 4, 2024

Conversation

cpvalente
Copy link
Owner

@cpvalente cpvalente commented Jul 11, 2024 •
edited
Loading

proposal for timeline view.
extracted here to make it easier to solve merge conflicts

TODO:

  • tweaks to UI
  • filter public
  • hide past

@coderabbitai coderabbitai
Copy link
Contributor

coderabbitai bot commented Jul 11, 2024 •
edited
Loading

Caution

Review failed

The pull request is closed.

Walkthrough

The updates integrate a comprehensive Timeline feature into the application, improving resource management and user interaction through lazy loading. Enhancements include new utility functions for color manipulation and time formatting, performance optimizations, and expanded translation support across multiple languages. New components for visualizing timelines and their entries further elevate the user experience, making project management more intuitive and visually appealing.

Changes

File Path Change Summary
apps/client/src/AppRouter.tsx Added lazy loading for the Timeline feature and its route; removed duplicate /public route.
apps/client/src/common/components/view-params-editor/ViewParamsEditor.tsx Suggested memoization for performance improvement.
apps/client/src/common/hooks/useSocket.ts Introduced useTimelineStatus function for extracting timeline properties.
apps/client/src/common/utils/styleUtils.ts Added alpha function for setting color opacity.
apps/client/src/common/utils/time.ts Introduced formatDuration function for formatting durations; refactored nowInMillis for clarity.
apps/client/src/features/operator/operator-event/OperatorEvent.module.scss Updated background color for .running class to $active-red.
apps/client/src/features/viewers/timeline/*.scss Added and modified styles for Timeline components and related elements.
apps/client/src/features/viewers/timeline/Timeline.tsx Introduced Timeline component utilizing hooks and utility functions, memoized for performance.
apps/client/src/features/viewers/timeline/TimelineEntry.tsx Added TimelineEntry component for individual timeline entries.
apps/client/src/features/viewers/timeline/TimelinePage.tsx Introduced TimelinePage component for displaying events.
apps/client/src/features/viewers/timeline/__tests__/timeline.utils.test.ts Added test cases for timeline utility functions.
apps/client/src/features/viewers/timeline/timeline-markers/*.scss Added styles for TimelineMarkers component.
apps/client/src/features/viewers/timeline/timeline-markers/TimelineMarkers.tsx Introduced TimelineMarkers component for visual markers.
apps/client/src/features/viewers/timeline/timeline-progress-bar/*.scss Added styles for ProgressBar component.
apps/client/src/features/viewers/timeline/timeline-progress-bar/TimelineProgressBar.tsx Introduced ProgressBar component for visualizing progress.
apps/client/src/features/viewers/timeline/timeline-section/*.scss Added styles for Section component.
apps/client/src/features/viewers/timeline/timeline-section/TimelineSection.tsx Introduced Section component for timeline categorization.
apps/client/src/features/viewers/timeline/timeline.options.ts Added getTimelineOptions function for view options.
apps/client/src/features/viewers/timeline/timeline.utils.ts Added utility functions for timeline operations and calculations.
apps/client/src/translation/languages/*.ts Added new translation keys for timeline features in various languages.
apps/client/src/viewerConfig.ts Added Timeline to navigatorConstants for navigation.
packages/utils/src/rundown-utils/rundownUtils.ts Added getEventWithId function for event retrieval.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant App
    participant TimelineComponent
    participant TimelineUtils

    User->>App: Navigate to /timeline
    App->>TimelineComponent: Render Timeline
    TimelineComponent->>TimelineUtils: Fetch timeline data
    TimelineUtils-->>TimelineComponent: Return timeline data
    TimelineComponent-->>User: Display timeline events
Loading 
sequenceDiagram
    participant User
    participant App
    participant TimelinePage

    User->>App: Navigate to TimelinePage
    App->>TimelinePage: Render TimelinePage
    TimelinePage-->>User: Display timeline with events
Loading

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?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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 as PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

coderabbitai[bot]
coderabbitai bot reviewed Jul 11, 2024
View reviewed changes
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 9

Outside diff range, codebase verification and nitpick comments (3)
apps/client/src/features/viewers/timeline/TimelinePage.tsx (1)

47-63: Ensure section titles are localized.

If the application supports multiple languages, ensure section titles are localized.

-  title: string;
+  title: ReactNode;
apps/client/src/common/utils/time.ts (1)

104-119: Suggestion: Improve the return format.

The return format can be improved for better readability by ensuring consistent formatting even if hours or minutes are zero.

-  if (hours > 0) {
-    result += `${hours}h `;
-  }
-  if (minutes > 0) {
-    result += `${minutes}m`;
-  }
+  result = `${hours}h ${minutes}m`;
apps/client/src/features/viewers/timeline/timeline.utils.ts (1)

26-28: Ensure proper edge case handling for getRelativePositionX.

The function getRelativePositionX does not handle the edge case where scheduleStart equals scheduleEnd, which leads to a division by zero. Additionally, there are no existing tests covering this scenario. Consider adding appropriate handling for this edge case and include tests to verify the function's behavior.

  • apps/client/src/features/viewers/timeline/timeline.utils.ts: Add edge case handling.
  • apps/client/src/features/viewers/timeline/__tests__/timeline.utils.test.ts: Add tests for the edge case.
Analysis chain

Ensure proper edge case handling for getRelativePositionX.

The function calculates the relative position of an element. Ensure it handles edge cases where scheduleStart equals scheduleEnd.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that `getRelativePositionX` handles edge cases.

# Test: Ensure the function handles cases where `scheduleStart` equals `scheduleEnd`.
rg --type ts -A 5 $'getRelativePositionX'

Length of output: 2083

Script:

#!/bin/bash
# Description: Verify that `getRelativePositionX` handles edge cases by inspecting test cases and usage.

# Search for test cases related to `getRelativePositionX`
rg --type ts -A 5 'getRelativePositionX'

# Search for test files that might contain tests for `TimelineProgressBar`
fd --type f --name '*.test.ts*'

Length of output: 2368

Script:

#!/bin/bash
# Description: Verify that `getRelativePositionX` handles edge cases by inspecting test cases and usage.

# Search for test cases related to `getRelativePositionX`
rg --type ts -A 5 'getRelativePositionX'

# Correct the fd command to search for test files
fd --type f -e test.ts -e test.tsx -e spec.ts -e spec.tsx

Length of output: 6048

Script:

#!/bin/bash
# Description: Verify that `getRelativePositionX` handles edge cases by inspecting test cases.

# Search for tests related to `getRelativePositionX` in `timeline.utils.test.ts`
rg 'getRelativePositionX' apps/client/src/features/viewers/timeline/__tests__/timeline.utils.test.ts

Length of output: 100

Script:

#!/bin/bash
# Description: Verify that `getRelativePositionX` handles edge cases by inspecting test cases.

# Search for tests in `TimelineProgressBar.test.tsx`
rg 'getRelativePositionX' apps/client/src/features/viewers/timeline/timeline-progress-bar/TimelineProgressBar.test.tsx

Length of output: 251

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 6bfa11d and 0c33b97.

Files selected for processing (19)
  • apps/client/src/AppRouter.tsx (3 hunks)
  • apps/client/src/common/components/view-params-editor/ViewParamsEditor.tsx (1 hunks)
  • apps/client/src/common/utils/styleUtils.ts (1 hunks)
  • apps/client/src/common/utils/time.ts (1 hunks)
  • apps/client/src/features/operator/operator-event/OperatorEvent.module.scss (2 hunks)
  • apps/client/src/features/viewers/timeline/Timeline.module.scss (1 hunks)
  • apps/client/src/features/viewers/timeline/Timeline.tsx (1 hunks)
  • apps/client/src/features/viewers/timeline/TimelineEntry.tsx (1 hunks)
  • apps/client/src/features/viewers/timeline/TimelinePage.module.scss (1 hunks)
  • apps/client/src/features/viewers/timeline/TimelinePage.tsx (1 hunks)
  • apps/client/src/features/viewers/timeline/tests/timeline.utils.test.ts (1 hunks)
  • apps/client/src/features/viewers/timeline/timeline-markers/TimelineMarkers.module.scss (1 hunks)
  • apps/client/src/features/viewers/timeline/timeline-markers/TimelineMarkers.tsx (1 hunks)
  • apps/client/src/features/viewers/timeline/timeline-progress-bar/TimelineProgressBar.module.scss (1 hunks)
  • apps/client/src/features/viewers/timeline/timeline-progress-bar/TimelineProgressBar.tsx (1 hunks)
  • apps/client/src/features/viewers/timeline/timeline.options.ts (1 hunks)
  • apps/client/src/features/viewers/timeline/timeline.utils.ts (1 hunks)
  • apps/client/src/theme/_ontimeStyles.scss (1 hunks)
  • apps/client/src/viewerConfig.ts (1 hunks)
Files skipped from review due to trivial changes (7)
  • apps/client/src/common/components/view-params-editor/ViewParamsEditor.tsx
  • apps/client/src/features/operator/operator-event/OperatorEvent.module.scss
  • apps/client/src/features/viewers/timeline/TimelinePage.module.scss
  • apps/client/src/features/viewers/timeline/timeline-markers/TimelineMarkers.module.scss
  • apps/client/src/features/viewers/timeline/timeline-progress-bar/TimelineProgressBar.module.scss
  • apps/client/src/theme/_ontimeStyles.scss
  • apps/client/src/viewerConfig.ts
Additional context used
GitHub Check: unit-test
apps/client/src/features/viewers/timeline/TimelineEntry.tsx

[failure] 1-1:
Run autofix to sort these imports!

Additional comments not posted (16)
apps/client/src/features/viewers/timeline/timeline.options.ts (1)

1-15: LGTM!

The getTimelineOptions function is correctly implemented and returns the expected array of ViewOption objects.

apps/client/src/features/viewers/timeline/timeline-markers/TimelineMarkers.tsx (1)

1-22: LGTM!

The TimelineMarkers component is correctly implemented and handles data fetching and rendering as expected.

apps/client/src/features/viewers/timeline/timeline-progress-bar/TimelineProgressBar.tsx (1)

1-22: LGTM!

The ProgressBar component is correctly implemented and handles time-based progress calculation and rendering as expected.

apps/client/src/features/viewers/timeline/Timeline.module.scss (1)

1-75: LGTM!

The SCSS styles for the timeline component are correctly implemented and ensure consistency and maintainability using variables and mixins.

apps/client/src/AppRouter.tsx (1)

19-19: LGTM! Ensure the correctness of the wrappers and route handling.

The integration of the Timeline component seems correct.

However, ensure that the wrappers withPreset and withData function as expected and the route /timeline is correctly handled.

Also applies to: 32-32, 56-56

apps/client/src/features/viewers/timeline/Timeline.tsx (2)

47-127: LGTM! Ensure the correctness of utility functions and improve readability.

The Timeline component seems to handle data rendering correctly.

However, ensure that the utility functions used are correctly implemented and tested. Consider breaking down large JSX blocks into smaller components for better readability.

19-41: LGTM! Ensure the correctness of the useRundown hook.

The useTimeline hook seems to handle data fetching and calculations correctly.

However, ensure that the useRundown hook is correctly implemented and tested.

apps/client/src/features/viewers/timeline/timeline.utils.ts (9)

33-37: LGTM! Ensure the correctness of getEstimatedWidth.

The function estimates the width of an element based on its content. Ensure it handles various content lengths correctly.

41-53: LGTM! Ensure the correctness of getElementPosition.

The function calculates the absolute position of an element based on a schedule. Ensure it handles edge cases correctly.

56-59: LGTM! Ensure the correctness of getStartHour.

The function calculates the start hour from a timestamp. Ensure it handles various timestamps correctly.

61-63: LGTM! Ensure the correctness of getEndHour.

The function calculates the end hour from a timestamp. Ensure it handles various timestamps correctly.

66-71: LGTM! Ensure the correctness of makeTimelineSections.

The function creates timeline sections based on start and end hours. Ensure it handles various hour ranges correctly.

75-90: LGTM! Ensure the correctness of getTimelineSections.

The function generates timeline sections based on rundown data. Ensure it handles various rundown data correctly.

97-107: LGTM! Ensure the correctness of getLaneLevel.

The function estimates the top offset for an element. Ensure it handles various element positions correctly.

112-122: LGTM! Ensure the correctness of getStatusLabel.

The function returns a formatted label for a progress status. Ensure it handles various statuses and time-to-start values correctly.

133-153: LGTM! Ensure the correctness of getUpcomingEvents.

The function returns upcoming events from the current event. Ensure it handles various event lists correctly.

@cpvalente cpvalente force-pushed the timeline-view branch 3 times, most recently from 3e7dfd9 to 90751b4 Compare July 11, 2024 12:31
coderabbitai[bot]
coderabbitai bot reviewed Jul 11, 2024
View reviewed changes
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

Outside diff range, codebase verification and nitpick comments (1)
apps/client/src/features/viewers/timeline/TimelineEntry.tsx (1)

9-24: Consider enhancing the TimelineEntry interface documentation.

Adding comments to the TimelineEntry interface properties would improve code readability and maintainability.

interface TimelineEntry {
  colour: string; // Color of the timeline entry
  duration: number; // Duration of the event in milliseconds
  isLast: boolean; // Flag indicating if this is the last entry
  lane: number; // The lane number where the entry is positioned
  left: number; // The left position of the entry in pixels
  status: ProgressStatus; // The status of the entry (finished, live, future)
  start: number; // Start time of the event in milliseconds
  title: string; // Title of the event
  width: number; // Width of the entry in pixels
  mayGrow: boolean; // Flag indicating if the entry may grow
  fullHeight: boolean; // Flag indicating if the entry should be full height
}
Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 6bfa11d and 3e7dfd9.

Files ignored due to path filters (1)
  • packages/utils/index.ts is excluded by none and included by none
Files selected for processing (22)
  • apps/client/src/AppRouter.tsx (3 hunks)
  • apps/client/src/common/components/view-params-editor/ViewParamsEditor.tsx (1 hunks)
  • apps/client/src/common/utils/styleUtils.ts (1 hunks)
  • apps/client/src/common/utils/time.ts (1 hunks)
  • apps/client/src/features/operator/operator-event/OperatorEvent.module.scss (2 hunks)
  • apps/client/src/features/viewers/timeline/Timeline.module.scss (1 hunks)
  • apps/client/src/features/viewers/timeline/Timeline.tsx (1 hunks)
  • apps/client/src/features/viewers/timeline/TimelineEntry.tsx (1 hunks)
  • apps/client/src/features/viewers/timeline/TimelinePage.module.scss (1 hunks)
  • apps/client/src/features/viewers/timeline/TimelinePage.tsx (1 hunks)
  • apps/client/src/features/viewers/timeline/tests/timeline.utils.test.ts (1 hunks)
  • apps/client/src/features/viewers/timeline/timeline-markers/TimelineMarkers.module.scss (1 hunks)
  • apps/client/src/features/viewers/timeline/timeline-markers/TimelineMarkers.tsx (1 hunks)
  • apps/client/src/features/viewers/timeline/timeline-progress-bar/TimelineProgressBar.module.scss (1 hunks)
  • apps/client/src/features/viewers/timeline/timeline-progress-bar/TimelineProgressBar.tsx (1 hunks)
  • apps/client/src/features/viewers/timeline/timeline-section/TimelineSection.module.scss (1 hunks)
  • apps/client/src/features/viewers/timeline/timeline-section/TimelineSection.tsx (1 hunks)
  • apps/client/src/features/viewers/timeline/timeline.options.ts (1 hunks)
  • apps/client/src/features/viewers/timeline/timeline.utils.ts (1 hunks)
  • apps/client/src/theme/_ontimeStyles.scss (1 hunks)
  • apps/client/src/viewerConfig.ts (1 hunks)
  • packages/utils/src/rundown-utils/rundownUtils.ts (1 hunks)
Files skipped from review due to trivial changes (8)
  • apps/client/src/common/components/view-params-editor/ViewParamsEditor.tsx
  • apps/client/src/features/viewers/timeline/TimelinePage.module.scss
  • apps/client/src/features/viewers/timeline/tests/timeline.utils.test.ts
  • apps/client/src/features/viewers/timeline/timeline-markers/TimelineMarkers.module.scss
  • apps/client/src/features/viewers/timeline/timeline-progress-bar/TimelineProgressBar.module.scss
  • apps/client/src/features/viewers/timeline/timeline-section/TimelineSection.module.scss
  • apps/client/src/theme/_ontimeStyles.scss
  • apps/client/src/viewerConfig.ts
Additional comments not posted (60)
apps/client/src/features/viewers/timeline/timeline.options.ts (2)

1-2: Imports look good.

The necessary modules are imported correctly.

4-15: Function getTimelineOptions looks good.

The function generates view options based on the timeFormat parameter. The options include a time option and a boolean option for full-height columns.

apps/client/src/features/viewers/timeline/timeline-markers/TimelineMarkers.tsx (2)

1-2: Imports look good.

The necessary modules and hooks are imported correctly.

6-22: Component TimelineMarkers looks good.

The component fetches data using the useRundown hook and generates timeline markers. The logic handles the case where data is not available or is in an invalid state.

apps/client/src/features/viewers/timeline/timeline-progress-bar/TimelineProgressBar.tsx (3)

1-2: Imports look good.

The necessary modules and hooks are imported correctly.

6-9: Interface ProgressBarProps looks good.

The interface defines the required properties for the TimelineProgressBar component.

11-22: Component TimelineProgressBar looks good.

The component fetches the current time using the useClock hook and calculates the progress bar width based on the start and end hours.

apps/client/src/features/viewers/timeline/timeline-section/TimelineSection.tsx (3)

1-2: Imports look good.

The necessary modules and types are imported correctly.

7-11: Interface SectionProps looks good.

The interface defines the required properties for the TimelineSection component.

13-23: Component TimelineSection looks good.

The component displays a section of the timeline with a title and content. It applies the appropriate styles based on the content and category.

apps/client/src/features/viewers/timeline/Timeline.module.scss (8)

1-10: LGTM!

The styles are straightforward and seem correct.

12-22: LGTM!

The styles are straightforward and seem correct.

24-31: LGTM!

The styles are straightforward and seem correct.

33-44: LGTM!

The styles are straightforward and seem correct.

46-49: LGTM!

The styles are straightforward and seem correct.

51-57: LGTM!

The styles are straightforward and seem correct.

59-66: LGTM!

The styles are straightforward and seem correct.

68-75: LGTM!

The styles are straightforward and seem correct.

apps/client/src/common/utils/styleUtils.ts (1)

38-49: LGTM!

The function is straightforward and seems correct.

apps/client/src/features/viewers/timeline/TimelinePage.tsx (1)

1-45: LGTM!

The component is straightforward and seems correct.

apps/client/src/features/operator/operator-event/OperatorEvent.module.scss (7)

Line range hint 1-33:
LGTM!

The styles are straightforward and seem correct.

Line range hint 35-49:
LGTM!

The styles are straightforward and seem correct.

Line range hint 51-55:
LGTM!

The styles are straightforward and seem correct.

Line range hint 57-61:
LGTM!

The styles are straightforward and seem correct.

Line range hint 63-66:
LGTM!

The styles are straightforward and seem correct.

Line range hint 68-71:
LGTM!

The styles are straightforward and seem correct.

Line range hint 73-92:
LGTM!

The styles are straightforward and seem correct.

apps/client/src/features/viewers/timeline/TimelineEntry.tsx (3)

1-3: Import statements are appropriate.

The imported hooks and utility functions are relevant for the component's functionality.

32-75: The TimelineEntry function component appears correct but ensure utility functions are used properly.

The component correctly uses props, utility functions, and CSS class combinations. However, consider adding PropTypes or TypeScript types for additional type safety.

78-90: The TimelineEntryStatus function component appears correct but ensure the hook usage is appropriate.

The component correctly uses the useClock hook and getStatusLabel utility function. Consider adding PropTypes or TypeScript types for additional type safety.

apps/client/src/common/utils/time.ts (5)

Line range hint 6-14:
The nowInMillis function appears correct but consider edge cases.

The function correctly calculates the current time in milliseconds since midnight. Consider adding tests for edge cases, such as during daylight saving time changes.

Line range hint 21-24:
The getFormatFromParams function appears correct but consider edge cases.

The function correctly retrieves the time format from the URL parameters. Ensure that it handles invalid or missing parameters gracefully.

Line range hint 29-32:
The getFormatFromSettings function appears correct but consider edge cases.

The function correctly retrieves the time format from the application settings. Ensure that it handles undefined settings gracefully.

Line range hint 72-97:
The formatTime function appears correct but consider edge cases.

The function correctly formats a given time in milliseconds into a string. Ensure that it handles negative and null values correctly.

104-120: The formatDuration function appears correct but consider edge cases.

The function correctly formats a duration time into a human-readable string. Ensure that it handles edge cases, such as very short or very long durations, correctly.

apps/client/src/AppRouter.tsx (4)

Line range hint 1-19:
The component imports and route definitions appear correct but ensure all components are imported accurately.

The imports and route definitions for various components are appropriate. Ensure that all components are imported correctly and are available for lazy loading.

32-32: The STimeline component is correctly wrapped with presets and data.

The wrapping of the Timeline component with withPreset and withData HOCs is appropriate.

48-56: The route definitions appear correct but ensure navigation is handled accurately.

The route definitions for various views, including the new timeline view, are appropriate. Ensure that navigation is handled correctly and all routes are accessible.

Line range hint 61-67:
The protected route definitions appear correct but ensure navigation is handled accurately.

The protected route definitions for various views are appropriate. Ensure that navigation is handled correctly and all routes are accessible.

apps/client/src/features/viewers/timeline/Timeline.tsx (2)

19-41: The useTimeline hook appears correct but ensure data processing logic is efficient.

The hook correctly retrieves and processes rundown data for the timeline view. Ensure that the data processing logic is efficient and handles edge cases correctly.

47-128: The Timeline function component appears correct but ensure utility functions are used properly.

The component correctly uses props, utility functions, and CSS class combinations. However, consider adding PropTypes or TypeScript types for additional type safety.

apps/client/src/features/viewers/timeline/timeline.utils.ts (7)

34-37: LGTM!

The function correctly estimates the width based on the content length.

57-59: LGTM!

The function correctly calculates the start hour based on the start time.

62-64: LGTM!

The function correctly calculates the end hour based on the end time.

67-72: LGTM!

The function correctly creates timeline sections based on the start and end hours.

98-107: LGTM!

The function correctly estimates the top offset for an element.

113-122: LGTM!

The function correctly returns a formatted label for a progress status.

134-153: Verify the correctness of getEventWithId.

The use of getEventWithId should be verified to ensure it correctly retrieves the event by ID.

Verification successful

The getEventWithId function correctly retrieves an event by its ID.

The implementations in both apps/server/src/services/rundown-service/rundownUtils.ts and packages/utils/src/rundown-utils/rundownUtils.ts are correct.

  • apps/server/src/services/rundown-service/rundownUtils.ts: Takes an eventId and returns the matching event from the rundown.
  • packages/utils/src/rundown-utils/rundownUtils.ts: Takes a rundown and an id, and returns the matching event from the provided rundown.

This confirms that the usage of getEventWithId in the getUpcomingEvents function is correct.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the correctness of `getEventWithId`.

# Test: Search for the function implementation and usage. Expect: Correct retrieval by ID.
rg --type ts -A 5 $'getEventWithId'

Length of output: 6954

Script:

#!/bin/bash
# Extract the implementation of `getEventWithId` from the identified files.

# Extract from `apps/server/src/services/rundown-service/rundownUtils.ts`
echo "Implementation in apps/server/src/services/rundown-service/rundownUtils.ts:"
rg --type ts -A 5 -B 5 $'export function getEventWithId' apps/server/src/services/rundown-service/rundownUtils.ts

# Extract from `packages/utils/src/rundown-utils/rundownUtils.ts`
echo "Implementation in packages/utils/src/rundown-utils/rundownUtils.ts:"
rg --type ts -A 5 -B 5 $'export function getEventWithId' packages/utils/src/rundown-utils/rundownUtils.ts

Length of output: 1007

packages/utils/src/rundown-utils/rundownUtils.ts (12)

Line range hint 9-11:
LGTM!

The function correctly returns the first event in the rundown.

Line range hint 15-17:
LGTM!

The function correctly returns the first event in a normalised rundown.

Line range hint 22-29:
LGTM!

The function correctly returns the first scheduled event in the rundown.

Line range hint 34-41:
LGTM!

The function correctly returns the first scheduled event in a normalised rundown.

Line range hint 46-49:
LGTM!

The function correctly returns the last event in a normalised rundown.

Line range hint 54-63:
LGTM!

The function correctly returns the last scheduled event in the rundown.

Line range hint 68-77:
LGTM!

The function correctly returns the last scheduled event in a normalised rundown.

Line range hint 82-89:
LGTM!

The function correctly returns the next entry in the rundown.

Line range hint 94-102:
LGTM!

The function correctly returns the next entry in a normalised rundown.

Line range hint 107-116:
LGTM!

The function correctly returns the next scheduled event in the rundown.

Line range hint 121-130:
LGTM!

The function correctly returns the next scheduled event in a normalised rundown.

330-332: LGTM!

The function correctly returns an event based on a provided ID.

coderabbitai[bot]
coderabbitai bot reviewed Jul 11, 2024
View reviewed changes
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

Outside diff range, codebase verification and nitpick comments (4)
apps/client/src/common/utils/styleUtils.ts (1)

38-49: Consider logging errors.

The alpha function appears correct, but it would be beneficial to log errors for debugging purposes.

-  } catch (_error) {
+  } catch (error) {
+    console.error('Error applying alpha to color:', error);
   }
apps/client/src/features/viewers/timeline/TimelinePage.tsx (1)

22-46: Consider addressing the TODO comment.

The TimelinePage component appears correct, but consider addressing the TODO comment about inheriting from viewWrapper.

apps/client/src/features/viewers/timeline/TimelineEntry.tsx (2)

32-75: Consider extracting inline styles to CSS classes.

The TimelineEntry component appears correct, but extracting inline styles to CSS classes would improve maintainability.

83-90: Consider addressing the TODO comment.

The TimelineEntryStatus component appears correct, but consider addressing the TODO comment about accounting for offset.

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 6bfa11d and 90751b4.

Files ignored due to path filters (1)
  • packages/utils/index.ts is excluded by none and included by none
Files selected for processing (22)
  • apps/client/src/AppRouter.tsx (3 hunks)
  • apps/client/src/common/components/view-params-editor/ViewParamsEditor.tsx (1 hunks)
  • apps/client/src/common/utils/styleUtils.ts (1 hunks)
  • apps/client/src/common/utils/time.ts (1 hunks)
  • apps/client/src/features/operator/operator-event/OperatorEvent.module.scss (2 hunks)
  • apps/client/src/features/viewers/timeline/Timeline.module.scss (1 hunks)
  • apps/client/src/features/viewers/timeline/Timeline.tsx (1 hunks)
  • apps/client/src/features/viewers/timeline/TimelineEntry.tsx (1 hunks)
  • apps/client/src/features/viewers/timeline/TimelinePage.module.scss (1 hunks)
  • apps/client/src/features/viewers/timeline/TimelinePage.tsx (1 hunks)
  • apps/client/src/features/viewers/timeline/tests/timeline.utils.test.ts (1 hunks)
  • apps/client/src/features/viewers/timeline/timeline-markers/TimelineMarkers.module.scss (1 hunks)
  • apps/client/src/features/viewers/timeline/timeline-markers/TimelineMarkers.tsx (1 hunks)
  • apps/client/src/features/viewers/timeline/timeline-progress-bar/TimelineProgressBar.module.scss (1 hunks)
  • apps/client/src/features/viewers/timeline/timeline-progress-bar/TimelineProgressBar.tsx (1 hunks)
  • apps/client/src/features/viewers/timeline/timeline-section/TimelineSection.module.scss (1 hunks)
  • apps/client/src/features/viewers/timeline/timeline-section/TimelineSection.tsx (1 hunks)
  • apps/client/src/features/viewers/timeline/timeline.options.ts (1 hunks)
  • apps/client/src/features/viewers/timeline/timeline.utils.ts (1 hunks)
  • apps/client/src/theme/_ontimeStyles.scss (1 hunks)
  • apps/client/src/viewerConfig.ts (1 hunks)
  • packages/utils/src/rundown-utils/rundownUtils.ts (1 hunks)
Files skipped from review due to trivial changes (8)
  • apps/client/src/common/components/view-params-editor/ViewParamsEditor.tsx
  • apps/client/src/features/operator/operator-event/OperatorEvent.module.scss
  • apps/client/src/features/viewers/timeline/TimelinePage.module.scss
  • apps/client/src/features/viewers/timeline/timeline-markers/TimelineMarkers.module.scss
  • apps/client/src/features/viewers/timeline/timeline-progress-bar/TimelineProgressBar.module.scss
  • apps/client/src/features/viewers/timeline/timeline-section/TimelineSection.module.scss
  • apps/client/src/theme/_ontimeStyles.scss
  • apps/client/src/viewerConfig.ts
Additional comments not posted (45)
apps/client/src/features/viewers/timeline/timeline.options.ts (1)

4-14: Add type annotations for better clarity and maintainability.

Enhancing the readability and maintainability of the function by adding type annotations.

-export const getTimelineOptions = (timeFormat: string): ViewOption[] => {
+export const getTimelineOptions = (timeFormat: string): ViewOption[] => {
  return [
    getTimeOption(timeFormat),
    {
      id: 'fullHeight',
      title: 'Full height columns',
      description: 'Whether the columns in the timeline should be the full height ',
      type: 'boolean',
      defaultValue: false,
    },
  ];
};

Likely invalid or redundant comment.

apps/client/src/features/viewers/timeline/timeline-markers/TimelineMarkers.tsx (1)

6-21: Add PropTypes or TypeScript interfaces for better type safety.

Enhancing the type safety of the component by adding PropTypes or TypeScript interfaces to define the expected props and their types.

+import PropTypes from 'prop-types';
...

+TimelineMarkers.propTypes = {
+  data: PropTypes.shape({
+    rundown: PropTypes.array.isRequired,
+    order: PropTypes.array.isRequired,
+    revision: PropTypes.number.isRequired,
+  }).isRequired,
+};

Likely invalid or redundant comment.

apps/client/src/features/viewers/timeline/Timeline.module.scss (8)

3-5: LGTM!

The .timeline class flex property appears correct.

7-10: LGTM!

The .timelineEvents class position and height properties appear correct.

12-22: LGTM!

The .entryColumn class properties and the nested .fullHeight class appear correct.

24-31: LGTM!

The .entryContent class properties and the nested .lastElement class appear correct.

33-44: LGTM!

The .entryText class properties and the nested .textBg class appear correct.

46-49: LGTM!

The .start and .title classes font-weight properties appear correct.

52-57: LGTM!

The [data-status='finished'] class properties and the nested .status class appear correct.

59-75: LGTM!

The [data-status='live'] and [data-status='future'] class properties and the nested .status classes appear correct.

apps/client/src/features/viewers/timeline/__tests__/timeline.utils.test.ts (8)

5-16: Test case for getElementPosition with one event looks good.

The test case correctly verifies the position and width for an event that spans the entire container.

18-28: Test case for getElementPosition with an event starting halfway looks good.

The test case correctly verifies the position and width for an event that starts halfway through the container.

30-40: Test case for getElementPosition with an event ending halfway looks good.

The test case correctly verifies the position and width for an event that ends halfway through the container.

42-53: Test case for getElementPosition with an event in the middle looks good.

The test case correctly verifies the position and width for an event that occurs in the middle of the timeline.

56-62: Test case for getLaneLevel with no overlap looks good.

The test case correctly verifies the lane assignment when there is no overlap.

64-69: Test case for getLaneLevel with overlap looks good.

The test case correctly verifies the lane assignment when there is overlap.

71-76: Test case for getLaneLevel with all lanes overlapped looks good.

The test case correctly verifies the lane assignment when all existing lanes are overlapped.

78-83: Test case for getLaneLevel for the first event looks good.

The test case correctly verifies the lane assignment for the first event in the timeline.

apps/client/src/common/utils/time.ts (1)

104-120: Function formatDuration looks good.

The function correctly formats a duration in milliseconds to a string in hours and minutes, handling non-positive durations gracefully.

apps/client/src/AppRouter.tsx (2)

19-20: Lazy-loading of Timeline looks good.

The Timeline component is correctly lazy-loaded and wrapped with withPreset and withData HOCs.

32-32: Addition of Timeline route looks good.

The Timeline route is correctly added to the application routes.

Also applies to: 56-56

apps/client/src/features/viewers/timeline/Timeline.tsx (2)

19-41: useTimeline hook looks good.

The hook correctly fetches rundown data and calculates the start and end hours for the timeline.

47-127: Timeline component looks good.

The component correctly renders the timeline with markers, progress bar, and events, handling various scenarios for event rendering.

apps/client/src/features/viewers/timeline/timeline.utils.ts (6)

34-38: LGTM!

The function logic for estimating width seems correct.

57-59: LGTM!

The function logic for calculating the start hour seems correct.

62-64: LGTM!

The function logic for calculating the end hour seems correct.

76-90: LGTM!

The function logic for creating timeline sections seems correct.

98-107: LGTM!

The function logic for estimating the top offset seems correct.

113-122: LGTM!

The function logic for returning a formatted label seems correct.

packages/utils/src/rundown-utils/rundownUtils.ts (16)

Line range hint 9-11:
LGTM!

The function logic for returning the first event seems correct.

Line range hint 14-16:
LGTM!

The function logic for returning the first event in a normalised rundown seems correct.

Line range hint 19-27:
LGTM!

The function logic for returning the first scheduled event seems correct.

Line range hint 30-38:
LGTM!

The function logic for returning the first scheduled event in a normalised rundown seems correct.

Line range hint 41-43:
LGTM!

The function logic for returning the last event in a normalised rundown seems correct.

Line range hint 46-54:
LGTM!

The function logic for returning the last scheduled event seems correct.

Line range hint 57-65:
LGTM!

The function logic for returning the last scheduled event in a normalised rundown seems correct.

Line range hint 68-74:
LGTM!

The function logic for returning the next entry seems correct.

Line range hint 77-83:
LGTM!

The function logic for returning the next entry in a normalised rundown seems correct.

Line range hint 86-96:
LGTM!

The function logic for returning the next scheduled event seems correct.

Line range hint 99-109:
LGTM!

The function logic for returning the next scheduled event in a normalised rundown seems correct.

Line range hint 112-118:
LGTM!

The function logic for returning the previous entry seems correct.

Line range hint 121-127:
LGTM!

The function logic for returning the previous entry in a normalised rundown seems correct.

Line range hint 130-140:
LGTM!

The function logic for returning the previous scheduled event seems correct.

Line range hint 143-153:
LGTM!

The function logic for returning the previous scheduled event in a normalised rundown seems correct.

330-332: LGTM!

The function logic for returning an event by its ID seems correct.

apps/client/src/features/viewers/timeline/timeline-progress-bar/TimelineProgressBar.tsx Outdated
Comment on lines 11 to 23
export default function ProgressBar(props: ProgressBarProps) {
const { startHour, endHour } = props;
const { clock } = useClock();

const width = getRelativePositionX(startHour, endHour, clock);

return (
<div className={style.progressBar}>
<div className={style.progress} style={{ width: `${width}%` }} />
</div>
);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add PropTypes or TypeScript interfaces for better type safety.

Enhancing the type safety of the component by adding PropTypes or TypeScript interfaces to define the expected props and their types.

+import PropTypes from 'prop-types';
...

+ProgressBar.propTypes = {
+  startHour: PropTypes.number.isRequired,
+  endHour: PropTypes.number.isRequired,
+};

Committable suggestion was skipped due to low confidence.

apps/client/src/features/viewers/timeline/timeline-section/TimelineSection.tsx
Comment on lines +13 to +23
export default function Section(props: SectionProps) {
const { category, content, title } = props;

const contentClasses = cx([style.sectionContent, content != null ? style[category] : style.subdue]);
return (
<div>
<div className={style.sectionTitle}>{title}</div>
<div className={contentClasses}>{content ?? '-'}</div>
</div>
);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add PropTypes or TypeScript interfaces for better type safety.

Enhancing the type safety of the component by adding PropTypes or TypeScript interfaces to define the expected props and their types.

+import PropTypes from 'prop-types';
...

+Section.propTypes = {
+  category: PropTypes.oneOf(['now', 'next']).isRequired,
+  content: PropTypes.string,
+  title: PropTypes.string.isRequired,
+};
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
export default function Section(props: SectionProps) {
const { category, content, title } = props;
const contentClasses = cx([style.sectionContent, content != null ? style[category] : style.subdue]);
return (
<div>
<div className={style.sectionTitle}>{title}</div>
<div className={contentClasses}>{content ?? '-'}</div>
</div>
);
}
import PropTypes from 'prop-types';
export default function Section(props: SectionProps) {
const { category, content, title } = props;
const contentClasses = cx([style.sectionContent, content != null ? style[category] : style.subdue]);
return (
<div>
<div className={style.sectionTitle}>{title}</div>
<div className={contentClasses}>{content ?? '-'}</div>
</div>
);
}
Section.propTypes = {
category: PropTypes.oneOf(['now', 'next']).isRequired,
content: PropTypes.string,
title: PropTypes.string.isRequired,
};

apps/client/src/features/viewers/timeline/timeline.utils.ts Outdated Show resolved Hide resolved
apps/client/src/features/viewers/timeline/timeline.utils.ts Show resolved Hide resolved
apps/client/src/features/viewers/timeline/timeline.utils.ts Outdated Show resolved Hide resolved
@cpvalente cpvalente requested a review from alex-Arc July 20, 2024 14:25
coderabbitai[bot]
coderabbitai bot reviewed Jul 20, 2024
View reviewed changes
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Inline review comments failed to post

Actionable comments posted: 10

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 90751b4 and 3010366.

Files selected for processing (16)
  • apps/client/src/common/hooks/useSocket.ts (1 hunks)
  • apps/client/src/features/viewers/timeline/Timeline.module.scss (1 hunks)
  • apps/client/src/features/viewers/timeline/TimelineEntry.tsx (1 hunks)
  • apps/client/src/features/viewers/timeline/TimelinePage.module.scss (1 hunks)
  • apps/client/src/features/viewers/timeline/TimelinePage.tsx (1 hunks)
  • apps/client/src/features/viewers/timeline/timeline-markers/TimelineMarkers.module.scss (1 hunks)
  • apps/client/src/features/viewers/timeline/timeline.utils.ts (1 hunks)
  • apps/client/src/translation/languages/de.ts (1 hunks)
  • apps/client/src/translation/languages/en.ts (1 hunks)
  • apps/client/src/translation/languages/es.ts (1 hunks)
  • apps/client/src/translation/languages/fr.ts (1 hunks)
  • apps/client/src/translation/languages/it.ts (1 hunks)
  • apps/client/src/translation/languages/no.ts (1 hunks)
  • apps/client/src/translation/languages/pl.ts (1 hunks)
  • apps/client/src/translation/languages/pt.ts (1 hunks)
  • apps/client/src/translation/languages/sv.ts (1 hunks)
Files skipped from review due to trivial changes (2)
  • apps/client/src/features/viewers/timeline/TimelinePage.module.scss
  • apps/client/src/features/viewers/timeline/timeline-markers/TimelineMarkers.module.scss
Additional comments not posted (40)
apps/client/src/translation/languages/no.ts (1)

22-25: New translations added for timeline feature.

The following translations have been added:

  • 'timeline.live': 'live'
  • 'timeline.finished': 'Ferdig'
  • 'timeline.pending': 'Venter'
  • 'timeline.followedby': 'Etterfulgt av'

These translations appear to be accurate and consistent with the existing translation patterns.

apps/client/src/translation/languages/en.ts (1)

20-23: New translations added for timeline feature.

The following translations have been added:

  • 'timeline.live': 'live'
  • 'timeline.finished': 'finished'
  • 'timeline.pending': 'pending'
  • 'timeline.followedby': 'Followed by'

These translations are accurate and consistent with the existing translation patterns.

apps/client/src/translation/languages/sv.ts (1)

22-25: New translations added for timeline feature.

The following translations have been added:

  • 'timeline.live': 'live'
  • 'timeline.finished': 'Avslutad'
  • 'timeline.pending': 'Väntande'
  • 'timeline.followedby': 'Följt av'

These translations are accurate and consistent with the existing translation patterns.

apps/client/src/translation/languages/fr.ts (4)

22-22: Translation entry timeline.live is appropriate.

The translation for "live" is correctly added as "live".

23-23: Translation entry timeline.finished is appropriate.

The translation for "finished" is correctly added as "Terminé".

24-24: Translation entry timeline.pending is appropriate.

The translation for "pending" is correctly added as "En attente".

25-25: Translation entry timeline.followedby is appropriate.

The translation for "followed by" is correctly added as "Suivi de".

apps/client/src/translation/languages/pl.ts (4)

22-22: Translation entry timeline.live is appropriate.

The translation for "live" is correctly added as "live".

23-23: Translation entry timeline.finished is appropriate.

The translation for "finished" is correctly added as "Zakończony".

24-24: Translation entry timeline.pending is appropriate.

The translation for "pending" is correctly added as "Oczekujący".

25-25: Translation entry timeline.followedby is appropriate.

The translation for "followed by" is correctly added as "Następnie".

apps/client/src/translation/languages/it.ts (4)

22-22: Translation entry timeline.live is appropriate.

The translation for "live" is correctly added as "live".

23-23: Translation entry timeline.finished is appropriate.

The translation for "finished" is correctly added as "Terminato".

24-24: Translation entry timeline.pending is appropriate.

The translation for "pending" is correctly added as "In sospeso".

25-25: Translation entry timeline.followedby is appropriate.

The translation for "followed by" is correctly added as "Seguito da".

apps/client/src/translation/languages/pt.ts (3)

23-23: Translation approved for 'timeline.finished'.

The translation for 'timeline.finished' is correct.

24-24: Translation approved for 'timeline.pending'.

The translation for 'timeline.pending' is correct.

25-25: Translation approved for 'timeline.followedby'.

The translation for 'timeline.followedby' is correct.

apps/client/src/translation/languages/es.ts (3)

23-23: Translation approved for 'timeline.finished'.

The translation for 'timeline.finished' is correct.

24-24: Translation approved for 'timeline.pending'.

The translation for 'timeline.pending' is correct.

25-25: Translation approved for 'timeline.followedby'.

The translation for 'timeline.followedby' is correct.

apps/client/src/translation/languages/de.ts (3)

23-23: Translation approved for 'timeline.finished'.

The translation for 'timeline.finished' is correct.

24-24: Translation approved for 'timeline.pending'.

The translation for 'timeline.pending' is correct.

25-25: Translation approved for 'timeline.followedby'.

The translation for 'timeline.followedby' is correct.

apps/client/src/features/viewers/timeline/Timeline.module.scss (9)

5-7: LGTM!

The .timeline class correctly uses flex: 1 to ensure it takes up available space within a flex container.

9-12: LGTM!

The .timelineEvents class correctly uses position: relative and height: 100% to ensure proper positioning and height.

14-21: LGTM!

The .entryColumn class correctly sets various properties for positioning, background color, borders, and minimum height. The use of CSS variables for colors enhances maintainability.

23-26: LGTM!

The .entryContent class correctly sets padding properties to ensure proper spacing within an entry column.

28-42: LGTM!

The .entryText class correctly sets various properties for positioning, color, padding, width, and white-space handling. The nested classes handle background color and white-space for the last element effectively.

45-48: LGTM!

The .start and .title classes correctly set the font-weight property to ensure these elements are displayed with a bold font weight.

51-56: LGTM!

The [data-status='finished'] class correctly sets the color property and hides the .status element to indicate finished events.

58-64: LGTM!

The [data-status='live'] class correctly sets the font-weight and color properties to ensure live events are displayed prominently. The status element is styled effectively with a red color.

67-73: LGTM!

The [data-status='future'] class correctly sets the font-weight and color properties to ensure future events are displayed appropriately. The status element is styled effectively with a green color.

apps/client/src/features/viewers/timeline/TimelinePage.tsx (1)

24-47: LGTM!

The TimelinePage component is well-structured and uses appropriate hooks and utility functions. The code changes are approved.

apps/client/src/features/viewers/timeline/TimelineEntry.tsx (2)

35-95: LGTM!

The TimelineEntry component is well-structured and uses appropriate hooks and utility functions. The use of useLayoutEffect for handling the right offset is appropriate. The code changes are approved.

103-118: LGTM!

The TimelineEntryStatus component is well-structured and uses appropriate hooks and utility functions. Isolating this component to avoid re-rendering too many elements is a good practice. The code changes are approved.

apps/client/src/features/viewers/timeline/timeline.utils.ts (3)

27-29: Consider adding parameter validation.

Ensure that scheduleStart, scheduleEnd, and now are valid numbers to prevent potential calculation errors.

+ if (isNaN(scheduleStart) || isNaN(scheduleEnd) || isNaN(now)) {
+   throw new Error('Invalid parameters');
+ }

42-54: Address the TODO comment.

The TODO comment indicates a potential issue with events that finish at midnight. Consider handling this case explicitly.

Do you want me to provide a fix for this issue or open a GitHub issue to track this task?

67-72: Consider adding parameter validation.

Ensure that firstHour and lastHour are valid numbers to prevent potential errors.

+ if (isNaN(firstHour) || isNaN(lastHour)) {
+   throw new Error('Invalid parameters');
+ }
apps/client/src/common/hooks/useSocket.ts (1)

187-194: Consider adding parameter validation.

Ensure that state is a valid object to prevent potential errors.

+ if (typeof state !== 'object') {
+   throw new Error('Invalid state');
+ }

Likely invalid or redundant comment.

Comments failed to post (10)
apps/client/src/translation/languages/pt.ts

22-22: Correct the translation for 'timeline.live'.

The translation for 'timeline.live' is still in English. The correct Portuguese translation should be 'ao vivo'.

-  'timeline.live': 'live',
+  'timeline.live': 'ao vivo',
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

  'timeline.live': 'ao vivo',
apps/client/src/features/viewers/timeline/timeline.utils.ts

34-37: Consider adding tests for this function.

Ensure that the function correctly estimates the width for various input strings.

Do you want me to generate the unit testing code or open a GitHub issue to track this task?

75-91: Address the TODO comment.

The TODO comment indicates a potential issue with elapsed days. Consider handling this case explicitly.

Do you want me to provide a fix for this issue or open a GitHub issue to track this task?

62-64: Consider adding parameter validation.

Ensure that endTime is a valid number to prevent potential calculation errors.

+ if (isNaN(endTime)) {
+   throw new Error('Invalid parameter');
+ }

Committable suggestion was skipped due to low confidence.

113-123: Consider adding parameter validation.

Ensure that timeToStart is a valid number and status is a valid string to prevent potential errors.

+ if (isNaN(timeToStart) || typeof status !== 'string') {
+   throw new Error('Invalid parameters');
+ }
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

export function getStatusLabel(timeToStart: number, status: ProgressStatus): string {
  if (isNaN(timeToStart) || typeof status !== 'string') {
    throw new Error('Invalid parameters');
  }

  if (status === 'finished' || status === 'live') {
    return status;
  }

  if (timeToStart < 0) {
    return 'pending';
  }

  return `T - ${formatDuration(timeToStart)}`;
}

57-59: Consider adding parameter validation.

Ensure that startTime is a valid number to prevent potential calculation errors.

+ if (isNaN(startTime)) {
+   throw new Error('Invalid parameter');
+ }
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

export function getStartHour(startTime: number): number {
  if (isNaN(startTime)) {
    throw new Error('Invalid parameter');
  }
  const hours = Math.floor(startTime / MILLIS_PER_HOUR);
  return hours;

98-107: Consider adding parameter validation.

Ensure that rightMostElements is a valid object and left is a valid number to prevent potential errors.

+ if (typeof rightMostElements !== 'object' || isNaN(left)) {
+   throw new Error('Invalid parameters');
+ }
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

export function getLaneLevel(rightMostElements: Record<number, number>, left: number): number {
  if (typeof rightMostElements !== 'object' || isNaN(left)) {
    throw new Error('Invalid parameters');
  }
  for (let checkLane = 0; checkLane < MAX_DEPTH; checkLane++) {
    if (rightMostElements[checkLane] === undefined) {
      return checkLane;
    }
    if (rightMostElements[checkLane] < left) {
      return checkLane;
    }
  }
  return 0;
apps/client/src/features/viewers/timeline/TimelinePage.tsx

23-23: Reminder: Address the TODO comment.

The TODO comment suggests that this view might need to inherit from viewWrapper.

Do you want me to investigate and address this TODO comment or open a GitHub issue to track this task?

apps/client/src/translation/languages/es.ts

22-22: Correct the translation for 'timeline.live'.

The translation for 'timeline.live' is still in English. The correct Spanish translation should be 'en vivo'.

-  'timeline.live': 'live',
+  'timeline.live': 'en vivo',
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

  'timeline.live': 'en vivo',
apps/client/src/translation/languages/de.ts

22-22: Correct the translation for 'timeline.live'.

The translation for 'timeline.live' is still in English. The correct German translation should be 'live' or 'direkt'.

-  'timeline.live': 'live',
+  'timeline.live': 'live',

Committable suggestion was skipped due to low confidence.

coderabbitai[bot]
coderabbitai bot reviewed Jul 20, 2024
View reviewed changes
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 3010366 and d076f75.

Files ignored due to path filters (1)
  • packages/utils/index.ts is excluded by none and included by none
Files selected for processing (32)
  • apps/client/src/AppRouter.tsx (3 hunks)
  • apps/client/src/common/components/view-params-editor/ViewParamsEditor.tsx (1 hunks)
  • apps/client/src/common/hooks/useSocket.ts (1 hunks)
  • apps/client/src/common/utils/styleUtils.ts (1 hunks)
  • apps/client/src/common/utils/time.ts (1 hunks)
  • apps/client/src/features/operator/operator-event/OperatorEvent.module.scss (2 hunks)
  • apps/client/src/features/viewers/timeline/Timeline.module.scss (1 hunks)
  • apps/client/src/features/viewers/timeline/Timeline.tsx (1 hunks)
  • apps/client/src/features/viewers/timeline/TimelineEntry.tsx (1 hunks)
  • apps/client/src/features/viewers/timeline/TimelinePage.module.scss (1 hunks)
  • apps/client/src/features/viewers/timeline/TimelinePage.tsx (1 hunks)
  • apps/client/src/features/viewers/timeline/tests/timeline.utils.test.ts (1 hunks)
  • apps/client/src/features/viewers/timeline/timeline-markers/TimelineMarkers.module.scss (1 hunks)
  • apps/client/src/features/viewers/timeline/timeline-markers/TimelineMarkers.tsx (1 hunks)
  • apps/client/src/features/viewers/timeline/timeline-progress-bar/TimelineProgressBar.module.scss (1 hunks)
  • apps/client/src/features/viewers/timeline/timeline-progress-bar/TimelineProgressBar.tsx (1 hunks)
  • apps/client/src/features/viewers/timeline/timeline-section/TimelineSection.module.scss (1 hunks)
  • apps/client/src/features/viewers/timeline/timeline-section/TimelineSection.tsx (1 hunks)
  • apps/client/src/features/viewers/timeline/timeline.options.ts (1 hunks)
  • apps/client/src/features/viewers/timeline/timeline.utils.ts (1 hunks)
  • apps/client/src/theme/_ontimeStyles.scss (1 hunks)
  • apps/client/src/translation/languages/de.ts (1 hunks)
  • apps/client/src/translation/languages/en.ts (1 hunks)
  • apps/client/src/translation/languages/es.ts (1 hunks)
  • apps/client/src/translation/languages/fr.ts (1 hunks)
  • apps/client/src/translation/languages/it.ts (1 hunks)
  • apps/client/src/translation/languages/no.ts (1 hunks)
  • apps/client/src/translation/languages/pl.ts (1 hunks)
  • apps/client/src/translation/languages/pt.ts (1 hunks)
  • apps/client/src/translation/languages/sv.ts (1 hunks)
  • apps/client/src/viewerConfig.ts (1 hunks)
  • packages/utils/src/rundown-utils/rundownUtils.ts (1 hunks)
Files skipped from review due to trivial changes (6)
  • apps/client/src/common/components/view-params-editor/ViewParamsEditor.tsx
  • apps/client/src/features/viewers/timeline/Timeline.module.scss
  • apps/client/src/features/viewers/timeline/timeline-markers/TimelineMarkers.module.scss
  • apps/client/src/features/viewers/timeline/timeline-progress-bar/TimelineProgressBar.module.scss
  • apps/client/src/features/viewers/timeline/timeline-section/TimelineSection.module.scss
  • apps/client/src/theme/_ontimeStyles.scss
Files skipped from review as they are similar to previous changes (14)
  • apps/client/src/common/hooks/useSocket.ts
  • apps/client/src/features/viewers/timeline/TimelineEntry.tsx
  • apps/client/src/features/viewers/timeline/TimelinePage.module.scss
  • apps/client/src/features/viewers/timeline/TimelinePage.tsx
  • apps/client/src/features/viewers/timeline/timeline.utils.ts
  • apps/client/src/translation/languages/de.ts
  • apps/client/src/translation/languages/en.ts
  • apps/client/src/translation/languages/es.ts
  • apps/client/src/translation/languages/fr.ts
  • apps/client/src/translation/languages/it.ts
  • apps/client/src/translation/languages/no.ts
  • apps/client/src/translation/languages/pl.ts
  • apps/client/src/translation/languages/pt.ts
  • apps/client/src/translation/languages/sv.ts
Additional comments not posted (22)
apps/client/src/features/viewers/timeline/timeline.options.ts (1)

1-2: Verify the usage of imported modules.

Ensure that getTimeOption and ViewOption are correctly imported and used.

Verification successful

Verification successful.

The imported modules getTimeOption and ViewOption are correctly used in the codebase.

  • getTimeOption is defined in apps/client/src/common/components/view-params-editor/constants.ts.
  • ViewOption is defined in apps/client/src/common/components/view-params-editor/types.ts.

Both are used appropriately in the file apps/client/src/features/viewers/timeline/timeline.options.ts.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the usage of `getTimeOption` and `ViewOption` in the codebase.

# Test: Search for the definitions of `getTimeOption` and `ViewOption`. Expect: Correct imports and usage.
rg --type ts -A 5 $'getTimeOption'
rg --type ts -A 5 $'ViewOption'

Length of output: 24961

apps/client/src/viewerConfig.ts (1)

6-6: LGTM! But verify the new navigation option usage.

The code changes are approved.

However, ensure that the new navigation option /timeline is correctly integrated and used in the application.

Verification successful

The new navigation option /timeline is correctly integrated and used in the application.

The /timeline route is defined in AppRouter.tsx and the relevant components are imported and used appropriately.

  • apps/client/src/viewerConfig.ts
  • apps/client/src/AppRouter.tsx
  • apps/client/src/features/viewers/timeline/TimelinePage.tsx
  • apps/client/src/features/viewers/timeline/Timeline.tsx
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the usage of the new navigation option `/timeline`.

# Test: Search for the usage of `/timeline` in the codebase. Expect: Correct integration and usage.
rg --type ts -A 5 $'/timeline'

Length of output: 5676

apps/client/src/features/viewers/timeline/timeline-markers/TimelineMarkers.tsx (1)

1-2: Verify the usage of imported modules.

Ensure that useRundown and getTimelineSections are correctly imported and used.

Verification successful

The usage of imported modules has been verified.

The useRundown hook and getTimelineSections function are correctly imported and used in the TimelineMarkers component.

  • useRundown is defined in apps/client/src/common/hooks-query/useRundown.ts and is used to fetch data.
  • getTimelineSections is defined in apps/client/src/features/viewers/timeline/timeline.utils.ts and is used to process the fetched data.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the usage of `useRundown` and `getTimelineSections` in the codebase.

# Test: Search for the definitions of `useRundown` and `getTimelineSections`. Expect: Correct imports and usage.
rg --type ts -A 5 $'useRundown'
rg --type ts -A 5 $'getTimelineSections'

Length of output: 11888

apps/client/src/features/viewers/timeline/timeline-progress-bar/TimelineProgressBar.tsx (2)

6-9: Good use of TypeScript interfaces for type safety.

The ProgressBarProps interface ensures that the component receives the correct prop types.

15-15: Verify the correctness of the width calculation logic.

Ensure that the getRelativePositionX function correctly calculates the width based on the current time, start hour, and end hour.

Verification successful

Verified: The width calculation logic in the getRelativePositionX function is correct.

  • The function normalizes the current time within the start and end hours and scales it to a percentage.
  • The result is clamped between 0 and 100 to ensure it stays within valid bounds.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the correctness of the `getRelativePositionX` function.

# Test: Search for the function implementation. Expect: Correct logic for width calculation.
rg --type ts --type tsx -A 5 $'function getRelativePositionX'

Length of output: 90

Script:

#!/bin/bash
# Description: Verify the correctness of the `getRelativePositionX` function.

# Test: Search for the function implementation. Expect: Correct logic for width calculation.
rg --type ts --type tsx --type javascript -A 10 'function getRelativePositionX'

Length of output: 109

Script:

#!/bin/bash
# Description: Verify the correctness of the `getRelativePositionX` function.

# Test: Search for the function implementation. Expect: Correct logic for width calculation.
rg -A 10 'function getRelativePositionX'

Length of output: 1199

apps/client/src/features/viewers/timeline/timeline-section/TimelineSection.tsx (2)

7-11: Good use of TypeScript interfaces for type safety.

The SectionProps interface ensures that the component receives the correct prop types.

16-16: Verify the correctness of the style application logic.

Ensure that the cx function and the style classes are applied correctly based on the content and category.

apps/client/src/common/utils/styleUtils.ts (1)

41-48: Good implementation of the alpha function for color manipulation.

The function correctly applies the specified opacity to the color using the Color library.

apps/client/src/features/operator/operator-event/OperatorEvent.module.scss (1)

33-33: Update background color for .running state.

The background color for the .running state has been updated to $active-red. Ensure this change aligns with the overall design guidelines and does not negatively impact the user experience.

apps/client/src/features/viewers/timeline/__tests__/timeline.utils.test.ts (1)

1-83: Comprehensive test coverage for getElementPosition and getLaneLevel.

The test cases cover various scenarios, ensuring the robustness of the utility functions. Consider adding edge cases if any are missing.

apps/client/src/common/utils/time.ts (1)

104-120: New function formatDuration added.

The function correctly formats durations into a human-readable string. The guard clause for non-positive durations is a good addition. Ensure this function is covered by unit tests.

apps/client/src/AppRouter.tsx (3)

19-19: Good practice: Lazy loading the Timeline component.

The lazy loading of the Timeline component is a good practice for performance optimization.

32-32: Good practice: Enhancing the Timeline component with higher-order components.

Wrapping the Timeline component with withPreset and withData higher-order components is a good pattern to enhance the component with additional functionalities.

56-56: New route definition for the Timeline component.

Adding a new route definition for the Timeline component allows navigation to the /timeline path.

apps/client/src/features/viewers/timeline/Timeline.tsx (6)

1-5: Necessary imports for the Timeline component.

The imports include React, hooks, utility functions, and styles necessary for the functionality of the Timeline component.

7-12: Necessary additional imports for the Timeline component.

The additional imports include hooks and utilities necessary for the functionality of the Timeline component.

17-17: Good practice: Memoizing the Timeline component.

Memoizing the Timeline component helps in performance optimization by preventing unnecessary re-renders.

19-41: Well-structured custom hook: useTimeline.

The useTimeline hook correctly handles data fetching and processing for the Timeline component. The logic is clear and well-structured.

43-45: Correctly defined TimelineProps interface.

The TimelineProps interface correctly defines the props for the Timeline component.

47-128: Well-structured Timeline component.

The Timeline component correctly handles rendering the timeline view based on the provided data and props. The logic is clear and well-structured.

packages/utils/src/rundown-utils/rundownUtils.ts (2)

330-332: Well-structured function: getEventWithId.

The getEventWithId function correctly searches for and returns an event by its ID in the rundown. The logic is clear and well-structured.

330-332: Exporting the getEventWithId function.

Exporting the getEventWithId function allows it to be used in other modules.

apps/client/src/features/viewers/timeline/timeline.options.ts Outdated Show resolved Hide resolved
apps/client/src/features/viewers/timeline/timeline-markers/TimelineMarkers.tsx Outdated Show resolved Hide resolved
apps/client/src/common/utils/styleUtils.ts Show resolved Hide resolved
alex-Arc
alex-Arc reviewed Jul 21, 2024
View reviewed changes
Copy link
Collaborator

@alex-Arc alex-Arc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

timerticks over midnight shows 25
image

coderabbitai[bot]
coderabbitai bot reviewed Jul 21, 2024
View reviewed changes
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between d076f75 and e90508b.

Files selected for processing (2)
  • apps/client/src/features/viewers/timeline/tests/timeline.utils.test.ts (1 hunks)
  • apps/client/src/features/viewers/timeline/timeline.utils.ts (1 hunks)
Files skipped from review as they are similar to previous changes (2)
  • apps/client/src/features/viewers/timeline/tests/timeline.utils.test.ts
  • apps/client/src/features/viewers/timeline/timeline.utils.ts

coderabbitai[bot]
coderabbitai bot reviewed Jul 22, 2024
View reviewed changes
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between e90508b and 0ab3ed8.

Files selected for processing (18)
  • apps/client/src/common/utils/time.ts (3 hunks)
  • apps/client/src/features/viewers/timeline/Timeline.module.scss (1 hunks)
  • apps/client/src/features/viewers/timeline/Timeline.tsx (1 hunks)
  • apps/client/src/features/viewers/timeline/TimelineEntry.tsx (1 hunks)
  • apps/client/src/features/viewers/timeline/TimelinePage.module.scss (1 hunks)
  • apps/client/src/features/viewers/timeline/TimelinePage.tsx (1 hunks)
  • apps/client/src/features/viewers/timeline/timeline-progress-bar/TimelineProgressBar.module.scss (1 hunks)
  • apps/client/src/features/viewers/timeline/timeline.options.ts (1 hunks)
  • apps/client/src/features/viewers/timeline/timeline.utils.ts (1 hunks)
  • apps/client/src/translation/languages/de.ts (1 hunks)
  • apps/client/src/translation/languages/en.ts (1 hunks)
  • apps/client/src/translation/languages/es.ts (1 hunks)
  • apps/client/src/translation/languages/fr.ts (1 hunks)
  • apps/client/src/translation/languages/it.ts (1 hunks)
  • apps/client/src/translation/languages/no.ts (1 hunks)
  • apps/client/src/translation/languages/pl.ts (1 hunks)
  • apps/client/src/translation/languages/pt.ts (1 hunks)
  • apps/client/src/translation/languages/sv.ts (1 hunks)
Files skipped from review due to trivial changes (1)
  • apps/client/src/features/viewers/timeline/TimelinePage.module.scss
Files skipped from review as they are similar to previous changes (15)
  • apps/client/src/features/viewers/timeline/Timeline.module.scss
  • apps/client/src/features/viewers/timeline/Timeline.tsx
  • apps/client/src/features/viewers/timeline/TimelineEntry.tsx
  • apps/client/src/features/viewers/timeline/TimelinePage.tsx
  • apps/client/src/features/viewers/timeline/timeline-progress-bar/TimelineProgressBar.module.scss
  • apps/client/src/features/viewers/timeline/timeline.utils.ts
  • apps/client/src/translation/languages/de.ts
  • apps/client/src/translation/languages/en.ts
  • apps/client/src/translation/languages/es.ts
  • apps/client/src/translation/languages/fr.ts
  • apps/client/src/translation/languages/it.ts
  • apps/client/src/translation/languages/no.ts
  • apps/client/src/translation/languages/pl.ts
  • apps/client/src/translation/languages/pt.ts
  • apps/client/src/translation/languages/sv.ts
Additional comments not posted (1)
apps/client/src/common/utils/time.ts (1)

12-22: LGTM!

The function nowInMillis correctly returns the current time in milliseconds and the use of constants improves clarity and maintainability.

@cpvalente cpvalente force-pushed the timeline-view branch 4 times, most recently from df7653e to 5a08b1b Compare July 22, 2024 20:20
coderabbitai[bot]
coderabbitai bot reviewed Jul 22, 2024
View reviewed changes
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Inline review comments failed to post

Actionable comments posted: 5

Outside diff range, codebase verification and nitpick comments (1)
apps/client/src/features/viewers/timeline/__tests__/timeline.utils.test.ts (1)

86-90: Fix the typo in the test description.

The test case makeTmelineSections creates an array between the hours given, end excluded has a typo in the description. It should be makeTimelineSections.

- describe('makeTmelineSections', () => {
+ describe('makeTimelineSections', () => {
Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 0ab3ed8 and df7653e.

Files ignored due to path filters (1)
  • packages/utils/index.ts is excluded by none and included by none
Files selected for processing (32)
  • apps/client/src/AppRouter.tsx (3 hunks)
  • apps/client/src/common/components/view-params-editor/ViewParamsEditor.tsx (1 hunks)
  • apps/client/src/common/hooks/useSocket.ts (1 hunks)
  • apps/client/src/common/utils/styleUtils.ts (1 hunks)
  • apps/client/src/common/utils/time.ts (3 hunks)
  • apps/client/src/features/operator/operator-event/OperatorEvent.module.scss (2 hunks)
  • apps/client/src/features/viewers/timeline/Timeline.module.scss (1 hunks)
  • apps/client/src/features/viewers/timeline/Timeline.tsx (1 hunks)
  • apps/client/src/features/viewers/timeline/TimelineEntry.tsx (1 hunks)
  • apps/client/src/features/viewers/timeline/TimelinePage.module.scss (1 hunks)
  • apps/client/src/features/viewers/timeline/TimelinePage.tsx (1 hunks)
  • apps/client/src/features/viewers/timeline/tests/timeline.utils.test.ts (1 hunks)
  • apps/client/src/features/viewers/timeline/timeline-markers/TimelineMarkers.module.scss (1 hunks)
  • apps/client/src/features/viewers/timeline/timeline-markers/TimelineMarkers.tsx (1 hunks)
  • apps/client/src/features/viewers/timeline/timeline-progress-bar/TimelineProgressBar.module.scss (1 hunks)
  • apps/client/src/features/viewers/timeline/timeline-progress-bar/TimelineProgressBar.tsx (1 hunks)
  • apps/client/src/features/viewers/timeline/timeline-section/TimelineSection.module.scss (1 hunks)
  • apps/client/src/features/viewers/timeline/timeline-section/TimelineSection.tsx (1 hunks)
  • apps/client/src/features/viewers/timeline/timeline.options.ts (1 hunks)
  • apps/client/src/features/viewers/timeline/timeline.utils.ts (1 hunks)
  • apps/client/src/theme/_ontimeStyles.scss (1 hunks)
  • apps/client/src/translation/languages/de.ts (1 hunks)
  • apps/client/src/translation/languages/en.ts (1 hunks)
  • apps/client/src/translation/languages/es.ts (1 hunks)
  • apps/client/src/translation/languages/fr.ts (1 hunks)
  • apps/client/src/translation/languages/it.ts (1 hunks)
  • apps/client/src/translation/languages/no.ts (1 hunks)
  • apps/client/src/translation/languages/pl.ts (1 hunks)
  • apps/client/src/translation/languages/pt.ts (1 hunks)
  • apps/client/src/translation/languages/sv.ts (1 hunks)
  • apps/client/src/viewerConfig.ts (1 hunks)
  • packages/utils/src/rundown-utils/rundownUtils.ts (1 hunks)
Files skipped from review due to trivial changes (8)
  • apps/client/src/features/operator/operator-event/OperatorEvent.module.scss
  • apps/client/src/features/viewers/timeline/Timeline.module.scss
  • apps/client/src/features/viewers/timeline/TimelinePage.module.scss
  • apps/client/src/features/viewers/timeline/timeline-progress-bar/TimelineProgressBar.module.scss
  • apps/client/src/features/viewers/timeline/timeline-progress-bar/TimelineProgressBar.tsx
  • apps/client/src/features/viewers/timeline/timeline-section/TimelineSection.module.scss
  • apps/client/src/features/viewers/timeline/timeline.options.ts
  • apps/client/src/translation/languages/no.ts
Files skipped from review as they are similar to previous changes (21)
  • apps/client/src/common/components/view-params-editor/ViewParamsEditor.tsx
  • apps/client/src/common/hooks/useSocket.ts
  • apps/client/src/common/utils/styleUtils.ts
  • apps/client/src/common/utils/time.ts
  • apps/client/src/features/viewers/timeline/Timeline.tsx
  • apps/client/src/features/viewers/timeline/TimelineEntry.tsx
  • apps/client/src/features/viewers/timeline/TimelinePage.tsx
  • apps/client/src/features/viewers/timeline/timeline-markers/TimelineMarkers.module.scss
  • apps/client/src/features/viewers/timeline/timeline-markers/TimelineMarkers.tsx
  • apps/client/src/features/viewers/timeline/timeline-section/TimelineSection.tsx
  • apps/client/src/features/viewers/timeline/timeline.utils.ts
  • apps/client/src/theme/_ontimeStyles.scss
  • apps/client/src/translation/languages/de.ts
  • apps/client/src/translation/languages/en.ts
  • apps/client/src/translation/languages/es.ts
  • apps/client/src/translation/languages/fr.ts
  • apps/client/src/translation/languages/it.ts
  • apps/client/src/translation/languages/pl.ts
  • apps/client/src/translation/languages/pt.ts
  • apps/client/src/viewerConfig.ts
  • packages/utils/src/rundown-utils/rundownUtils.ts
Additional context used
GitHub Check: unit-test
apps/client/src/features/viewers/timeline/__tests__/timeline.utils.test.ts

[failure] 60-60: src/features/viewers/timeline/tests/timeline.utils.test.ts > getLaneLevel() > should place in lane 0 if there is no overlap
TypeError: getLaneLevel is not a function
❯ src/features/viewers/timeline/tests/timeline.utils.test.ts:60:20

[failure] 67-67: src/features/viewers/timeline/tests/timeline.utils.test.ts > getLaneLevel() > should place in next available lane if there is overlap
TypeError: getLaneLevel is not a function
❯ src/features/viewers/timeline/tests/timeline.utils.test.ts:67:20

[failure] 74-74: src/features/viewers/timeline/tests/timeline.utils.test.ts > getLaneLevel() > should create a new lane if all existing lanes are overlapped
TypeError: getLaneLevel is not a function
❯ src/features/viewers/timeline/tests/timeline.utils.test.ts:74:20

[failure] 81-81: src/features/viewers/timeline/tests/timeline.utils.test.ts > getLaneLevel() > should place in lane 0 if it is the first event
TypeError: getLaneLevel is not a function
❯ src/features/viewers/timeline/tests/timeline.utils.test.ts:81:20

Additional comments not posted (9)
apps/client/src/translation/languages/sv.ts (3)

23-23: LGTM!

The translation entry timeline.done is correct.

24-24: LGTM!

The translation entry timeline.due is correct.

25-25: LGTM!

The translation entry timeline.followedby is correct.

apps/client/src/features/viewers/timeline/__tests__/timeline.utils.test.ts (5)

5-16: LGTM!

The test case getCSSPosition() accounts for rundown with one event is correct.

18-28: LGTM!

The test case getCSSPosition() accounts for an event that starts halfway and ends at end is correct.

30-40: LGTM!

The test case getCSSPosition() accounts for an event that starts first and ends halfway is correct.

42-53: LGTM!

The test case getCSSPosition() accounts for an event that is in the middle of the rundown is correct.

92-95: LGTM!

The test case makeTimelineSections wraps around midnight is correct.

apps/client/src/AppRouter.tsx (1)

19-19: LGTM!

The route definition for the Timeline component is correct and follows best practices.

Also applies to: 32-32, 56-56

Comments failed to post (5)
apps/client/src/translation/languages/sv.ts

22-22: Translate the term 'live' to Swedish.

The translation entry timeline.live should be in Swedish for consistency.

-  'timeline.live': 'live',
+  'timeline.live': 'Direkt',
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

  'timeline.live': 'Direkt',
apps/client/src/features/viewers/timeline/__tests__/timeline.utils.test.ts

78-83: Fix the import or definition of getLaneLevel.

The test case getLaneLevel() should place in lane 0 if it is the first event fails because getLaneLevel is not a function. Ensure that the function is correctly imported or defined.

- import { getLaneLevel } from '../timeline.utils';
+ import { getLaneLevel } from '../path/to/correct/module';

Committable suggestion was skipped due to low confidence.

Tools
GitHub Check: unit-test

[failure] 81-81: src/features/viewers/timeline/tests/timeline.utils.test.ts > getLaneLevel() > should place in lane 0 if it is the first event
TypeError: getLaneLevel is not a function
❯ src/features/viewers/timeline/tests/timeline.utils.test.ts:81:20

71-76: Fix the import or definition of getLaneLevel.

The test case getLaneLevel() should create a new lane if all existing lanes are overlapped fails because getLaneLevel is not a function. Ensure that the function is correctly imported or defined.

- import { getLaneLevel } from '../timeline.utils';
+ import { getLaneLevel } from '../path/to/correct/module';

Committable suggestion was skipped due to low confidence.

Tools
GitHub Check: unit-test

[failure] 74-74: src/features/viewers/timeline/tests/timeline.utils.test.ts > getLaneLevel() > should create a new lane if all existing lanes are overlapped
TypeError: getLaneLevel is not a function
❯ src/features/viewers/timeline/tests/timeline.utils.test.ts:74:20

56-62: Fix the import or definition of getLaneLevel.

The test case getLaneLevel() should place in lane 0 if there is no overlap fails because getLaneLevel is not a function. Ensure that the function is correctly imported or defined.

- import { getLaneLevel } from '../timeline.utils';
+ import { getLaneLevel } from '../path/to/correct/module';

Committable suggestion was skipped due to low confidence.

Tools
GitHub Check: unit-test

[failure] 60-60: src/features/viewers/timeline/tests/timeline.utils.test.ts > getLaneLevel() > should place in lane 0 if there is no overlap
TypeError: getLaneLevel is not a function
❯ src/features/viewers/timeline/tests/timeline.utils.test.ts:60:20

64-69: Fix the import or definition of getLaneLevel.

The test case getLaneLevel() should place in next available lane if there is overlap fails because getLaneLevel is not a function. Ensure that the function is correctly imported or defined.

- import { getLaneLevel } from '../timeline.utils';
+ import { getLaneLevel } from '../path/to/correct/module';

Committable suggestion was skipped due to low confidence.

Tools
GitHub Check: unit-test

[failure] 67-67: src/features/viewers/timeline/tests/timeline.utils.test.ts > getLaneLevel() > should place in next available lane if there is overlap
TypeError: getLaneLevel is not a function
❯ src/features/viewers/timeline/tests/timeline.utils.test.ts:67:20

@cpvalente cpvalente force-pushed the timeline-view branch 3 times, most recently from f8f1006 to 2105a2a Compare July 22, 2024 20:31
coderabbitai[bot]
coderabbitai bot reviewed Jul 22, 2024
View reviewed changes
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Inline review comments failed to post

Actionable comments posted: 2

Outside diff range, codebase verification and nitpick comments (1)
packages/utils/src/rundown-utils/rundownUtils.ts (1)

330-332: LGTM! Consider adding a JSDoc comment.

The function is straightforward and correctly uses the find method to locate an event by its ID.

For consistency with the rest of the file, consider adding a JSDoc comment.

+/**
+ * Gets an event by its ID from the rundown
+ * @param {OntimeRundown} rundown
+ * @param {string} id
+ * @return {OntimeRundownEntry | undefined}
+ */
export function getEventWithId(rundown: OntimeRundown, id: string): OntimeRundownEntry | undefined {
  return rundown.find((event) => event.id === id);
}
Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between df7653e and f8f1006.

Files ignored due to path filters (1)
  • packages/utils/index.ts is excluded by none and included by none
Files selected for processing (32)
  • apps/client/src/AppRouter.tsx (3 hunks)
  • apps/client/src/common/components/view-params-editor/ViewParamsEditor.tsx (1 hunks)
  • apps/client/src/common/hooks/useSocket.ts (1 hunks)
  • apps/client/src/common/utils/styleUtils.ts (1 hunks)
  • apps/client/src/common/utils/time.ts (3 hunks)
  • apps/client/src/features/operator/operator-event/OperatorEvent.module.scss (2 hunks)
  • apps/client/src/features/viewers/timeline/Timeline.module.scss (1 hunks)
  • apps/client/src/features/viewers/timeline/Timeline.tsx (1 hunks)
  • apps/client/src/features/viewers/timeline/TimelineEntry.tsx (1 hunks)
  • apps/client/src/features/viewers/timeline/TimelinePage.module.scss (1 hunks)
  • apps/client/src/features/viewers/timeline/TimelinePage.tsx (1 hunks)
  • apps/client/src/features/viewers/timeline/tests/timeline.utils.test.ts (1 hunks)
  • apps/client/src/features/viewers/timeline/timeline-markers/TimelineMarkers.module.scss (1 hunks)
  • apps/client/src/features/viewers/timeline/timeline-markers/TimelineMarkers.tsx (1 hunks)
  • apps/client/src/features/viewers/timeline/timeline-progress-bar/TimelineProgressBar.module.scss (1 hunks)
  • apps/client/src/features/viewers/timeline/timeline-progress-bar/TimelineProgressBar.tsx (1 hunks)
  • apps/client/src/features/viewers/timeline/timeline-section/TimelineSection.module.scss (1 hunks)
  • apps/client/src/features/viewers/timeline/timeline-section/TimelineSection.tsx (1 hunks)
  • apps/client/src/features/viewers/timeline/timeline.options.ts (1 hunks)
  • apps/client/src/features/viewers/timeline/timeline.utils.ts (1 hunks)
  • apps/client/src/theme/_ontimeStyles.scss (1 hunks)
  • apps/client/src/translation/languages/de.ts (1 hunks)
  • apps/client/src/translation/languages/en.ts (1 hunks)
  • apps/client/src/translation/languages/es.ts (1 hunks)
  • apps/client/src/translation/languages/fr.ts (1 hunks)
  • apps/client/src/translation/languages/it.ts (1 hunks)
  • apps/client/src/translation/languages/no.ts (1 hunks)
  • apps/client/src/translation/languages/pl.ts (1 hunks)
  • apps/client/src/translation/languages/pt.ts (1 hunks)
  • apps/client/src/translation/languages/sv.ts (1 hunks)
  • apps/client/src/viewerConfig.ts (1 hunks)
  • packages/utils/src/rundown-utils/rundownUtils.ts (1 hunks)
Files skipped from review due to trivial changes (8)
  • apps/client/src/features/operator/operator-event/OperatorEvent.module.scss
  • apps/client/src/features/viewers/timeline/Timeline.module.scss
  • apps/client/src/features/viewers/timeline/TimelinePage.module.scss
  • apps/client/src/features/viewers/timeline/timeline-markers/TimelineMarkers.module.scss
  • apps/client/src/features/viewers/timeline/timeline-section/TimelineSection.module.scss
  • apps/client/src/features/viewers/timeline/timeline.options.ts
  • apps/client/src/theme/_ontimeStyles.scss
  • apps/client/src/translation/languages/no.ts
Files skipped from review as they are similar to previous changes (20)
  • apps/client/src/AppRouter.tsx
  • apps/client/src/common/components/view-params-editor/ViewParamsEditor.tsx
  • apps/client/src/common/hooks/useSocket.ts
  • apps/client/src/features/viewers/timeline/Timeline.tsx
  • apps/client/src/features/viewers/timeline/TimelineEntry.tsx
  • apps/client/src/features/viewers/timeline/TimelinePage.tsx
  • apps/client/src/features/viewers/timeline/tests/timeline.utils.test.ts
  • apps/client/src/features/viewers/timeline/timeline-markers/TimelineMarkers.tsx
  • apps/client/src/features/viewers/timeline/timeline-progress-bar/TimelineProgressBar.module.scss
  • apps/client/src/features/viewers/timeline/timeline-progress-bar/TimelineProgressBar.tsx
  • apps/client/src/features/viewers/timeline/timeline-section/TimelineSection.tsx
  • apps/client/src/features/viewers/timeline/timeline.utils.ts
  • apps/client/src/translation/languages/en.ts
  • apps/client/src/translation/languages/es.ts
  • apps/client/src/translation/languages/fr.ts
  • apps/client/src/translation/languages/it.ts
  • apps/client/src/translation/languages/pl.ts
  • apps/client/src/translation/languages/pt.ts
  • apps/client/src/translation/languages/sv.ts
  • apps/client/src/viewerConfig.ts
Additional comments not posted (5)
apps/client/src/translation/languages/de.ts (4)

22-22: Translation entry timeline.live looks good.

The translation entry is correct and consistent with the existing format.

23-23: Translation entry timeline.done looks good.

The translation entry is correct and consistent with the existing format.

24-24: Translation entry timeline.due looks good.

The translation entry is correct and consistent with the existing format.

25-25: Translation entry timeline.followedby looks good.

The translation entry is correct and consistent with the existing format.

apps/client/src/common/utils/time.ts (1)

12-22: Modification to nowInMillis looks good.

The changes improve clarity and maintainability.

Comments failed to post (2)
apps/client/src/common/utils/styleUtils.ts

41-47: Improve error handling in the alpha function.

Currently, errors are silently ignored. Consider logging the error or providing a fallback mechanism.

-  } catch (_error) {
-    /* we do not handle errors here */
+  } catch (error) {
+    console.error('Error applying alpha to color:', error);
+    // Optionally, provide a fallback mechanism here
  }
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

export function alpha(colour: string, amount: number): string {
  try {
    const withAlpha = Color(colour).alpha(amount).hexa();
    return withAlpha;
  } catch (error) {
    console.error('Error applying alpha to color:', error);
    // Optionally, provide a fallback mechanism here
  }
apps/client/src/common/utils/time.ts

104-120: Improve guard clause for negative durations.

The guard clause currently handles non-positive durations but could be more explicit about handling negative durations.

-  if (duration <= 0) {
+  if (duration < 0) {
+    console.warn('Negative duration detected');
+    return '0h 0m';
+  }
+  if (duration === 0) {
    return '0h 0m';
  }
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

export function formatDuration(duration: number): string {
  // durations should never be negative, we handle it here to flag if there is an issue in future
  if (duration < 0) {
    console.warn('Negative duration detected');
    return '0h 0m';
  }
  if (duration === 0) {
    return '0h 0m';
  }

  const hours = Math.floor(duration / MILLIS_PER_HOUR);
  const minutes = Math.floor((duration % MILLIS_PER_HOUR) / MILLIS_PER_MINUTE);
  let result = '';
  if (hours > 0) {
    result += `${hours}h `;
  }
  if (minutes > 0) {
    result += `${minutes}m`;
  }
  return result;
}

coderabbitai[bot]
coderabbitai bot reviewed Aug 1, 2024
View reviewed changes
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

Outside diff range, codebase verification and nitpick comments (2)
apps/client/src/translation/languages/es.ts (1)

22-22: Correct translation for 'timeline.live'.

The translation for 'timeline.live' is correct but should be capitalized for consistency with other entries.

-  'timeline.live': 'live',
+  'timeline.live': 'En vivo',
apps/client/src/translation/languages/de.ts (1)

22-22: Correct translation for 'timeline.live'.

The translation for 'timeline.live' is correct but should be capitalized for consistency with other entries.

-  'timeline.live': 'live',
+  'timeline.live': 'Live',
Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between f8f1006 and 89d087c.

Files ignored due to path filters (6)
  • apps/cli/package.json is excluded by none and included by none
  • apps/client/package.json is excluded by none and included by none
  • apps/electron/package.json is excluded by none and included by none
  • apps/server/package.json is excluded by none and included by none
  • package.json is excluded by none and included by none
  • packages/utils/index.ts is excluded by none and included by none
Files selected for processing (33)
  • apps/client/src/AppRouter.tsx (3 hunks)
  • apps/client/src/common/components/view-params-editor/ViewParamsEditor.tsx (1 hunks)
  • apps/client/src/common/hooks/useSocket.ts (1 hunks)
  • apps/client/src/common/utils/styleUtils.ts (1 hunks)
  • apps/client/src/common/utils/time.ts (3 hunks)
  • apps/client/src/features/operator/operator-event/OperatorEvent.module.scss (2 hunks)
  • apps/client/src/features/viewers/timeline/Timeline.module.scss (1 hunks)
  • apps/client/src/features/viewers/timeline/Timeline.tsx (1 hunks)
  • apps/client/src/features/viewers/timeline/TimelineEntry.tsx (1 hunks)
  • apps/client/src/features/viewers/timeline/TimelinePage.module.scss (1 hunks)
  • apps/client/src/features/viewers/timeline/TimelinePage.tsx (1 hunks)
  • apps/client/src/features/viewers/timeline/tests/timeline.utils.test.ts (1 hunks)
  • apps/client/src/features/viewers/timeline/timeline-markers/TimelineMarkers.module.scss (1 hunks)
  • apps/client/src/features/viewers/timeline/timeline-markers/TimelineMarkers.tsx (1 hunks)
  • apps/client/src/features/viewers/timeline/timeline-progress-bar/TimelineProgressBar.module.scss (1 hunks)
  • apps/client/src/features/viewers/timeline/timeline-progress-bar/TimelineProgressBar.tsx (1 hunks)
  • apps/client/src/features/viewers/timeline/timeline-section/TimelineSection.module.scss (1 hunks)
  • apps/client/src/features/viewers/timeline/timeline-section/TimelineSection.tsx (1 hunks)
  • apps/client/src/features/viewers/timeline/timeline.options.ts (1 hunks)
  • apps/client/src/features/viewers/timeline/timeline.utils.ts (1 hunks)
  • apps/client/src/theme/_ontimeColours.scss (1 hunks)
  • apps/client/src/theme/_ontimeStyles.scss (1 hunks)
  • apps/client/src/translation/languages/de.ts (1 hunks)
  • apps/client/src/translation/languages/en.ts (1 hunks)
  • apps/client/src/translation/languages/es.ts (1 hunks)
  • apps/client/src/translation/languages/fr.ts (1 hunks)
  • apps/client/src/translation/languages/it.ts (1 hunks)
  • apps/client/src/translation/languages/no.ts (1 hunks)
  • apps/client/src/translation/languages/pl.ts (1 hunks)
  • apps/client/src/translation/languages/pt.ts (1 hunks)
  • apps/client/src/translation/languages/sv.ts (1 hunks)
  • apps/client/src/viewerConfig.ts (1 hunks)
  • packages/utils/src/rundown-utils/rundownUtils.ts (1 hunks)
Files skipped from review due to trivial changes (6)
  • apps/client/src/common/components/view-params-editor/ViewParamsEditor.tsx
  • apps/client/src/features/viewers/timeline/timeline-markers/TimelineMarkers.module.scss
  • apps/client/src/features/viewers/timeline/timeline-progress-bar/TimelineProgressBar.module.scss
  • apps/client/src/features/viewers/timeline/timeline-section/TimelineSection.module.scss
  • apps/client/src/theme/_ontimeStyles.scss
  • apps/client/src/viewerConfig.ts
Additional comments not posted (67)
apps/client/src/features/viewers/timeline/timeline.options.ts (1)

4-6: LGTM!

The code changes are approved.

apps/client/src/features/viewers/timeline/TimelinePage.module.scss (3)

1-11: LGTM!

The styles for the .timeline class are appropriate and well-defined.

13-16: LGTM!

The styles for the .title class are appropriate and well-defined.

18-24: LGTM!

The styles for the .sections class are appropriate and well-defined.

apps/client/src/features/viewers/timeline/timeline-markers/TimelineMarkers.tsx (1)

6-22: LGTM!

The code changes are approved.

apps/client/src/features/viewers/timeline/timeline-progress-bar/TimelineProgressBar.tsx (3)

1-4: Imports look good.

The imported modules are relevant and necessary for the component's functionality.

6-9: Interface definition is correct.

The ProgressBarProps interface correctly defines the expected props.

11-21: Component implementation looks good.

The ProgressBar component correctly uses the props and the custom hook to calculate the width of the progress bar.

apps/client/src/features/viewers/timeline/timeline-section/TimelineSection.tsx (3)

1-5: Imports look good.

The imported modules are relevant and necessary for the component's functionality.

7-11: Interface definition is correct.

The SectionProps interface correctly defines the expected props.

13-22: Component implementation looks good.

The Section component correctly uses the props and utility function to apply CSS classes conditionally.

apps/client/src/translation/languages/no.ts (2)

Line range hint 1-21:
Existing translations look good.

The existing translations cover common terms and countdown-related terms appropriately.

22-25: New translations look good.

The new translations for the timeline feature are appropriate and correctly localized.

apps/client/src/translation/languages/en.ts (1)

20-23: New translation entries look good.

The new entries for the timeline feature are correctly added and consistent with the existing translation strings.

apps/client/src/translation/languages/fr.ts (1)

22-25: New translation entries look good.

The new entries for the timeline feature are correctly added and consistent with the existing translation strings.

apps/client/src/translation/languages/sv.ts (1)

22-25: New translation entries look good.

The new entries for the timeline feature are correctly added and consistent with the existing translation strings.

apps/client/src/translation/languages/pl.ts (1)

22-25: Translations look good.

The new translation entries for the timeline feature are correctly added and consistent with the existing translation style.

apps/client/src/translation/languages/it.ts (1)

22-25: Translations look good.

The new translation entries for the timeline feature are correctly added and consistent with the existing translation style.

apps/client/src/translation/languages/pt.ts (1)

22-25: Translations look good.

The new translation entries for the timeline feature are correctly added and consistent with the existing translation style.

apps/client/src/translation/languages/es.ts (3)

23-23: Correct translation for 'timeline.done'.

The translation for 'timeline.done' is correct and consistent.

24-24: Correct translation for 'timeline.due'.

The translation for 'timeline.due' is correct and consistent.

25-25: Correct translation for 'timeline.followedby'.

The translation for 'timeline.followedby' is correct and consistent.

apps/client/src/translation/languages/de.ts (3)

23-23: Correct translation for 'timeline.done'.

The translation for 'timeline.done' is correct and consistent.

24-24: Correct translation for 'timeline.due'.

The translation for 'timeline.due' is correct and consistent.

25-25: Correct translation for 'timeline.followedby'.

The translation for 'timeline.followedby' is correct and consistent.

apps/client/src/features/operator/operator-event/OperatorEvent.module.scss (2)

33-33: LGTM! The background color change improves consistency.

The change to use $active-red for the .running state background color is consistent with the use of CSS variables for theming.

Line range hint 19-19: LGTM! The formatting adjustment improves readability.

The removal of the empty line before the .field class definition enhances the overall readability of the code.

apps/client/src/features/viewers/timeline/Timeline.module.scss (5)

3-6: LGTM! The variable declarations improve maintainability.

The declarations for $timeline-entry-height, $lane-height, $distance-from-timeline, and $timeline-height centralize configuration values, enhancing maintainability.

8-12: LGTM! The .timeline class styles are appropriate.

The styles ensure the timeline component occupies available space and has a consistent appearance.

14-18: LGTM! The .timelineEvents class styles are appropriate.

The styles ensure timeline events are positioned relative to the timeline.

20-38: LGTM! The .column class styles are appropriate.

The styles ensure columns within the timeline are positioned and styled correctly.

41-113: LGTM! The styles for various classes are appropriate.

The styles ensure the timeline view is displayed correctly, with appropriate handling for small areas, hidden elements, content layout, delays, time overview, and crossed-out text.

apps/client/src/theme/_ontimeColours.scss (1)

10-10: LGTM! The new color variable enhances the color palette.

The addition of $white-40 provides a semi-transparent white option, enhancing flexibility in UI design.

apps/client/src/features/viewers/timeline/__tests__/timeline.utils.test.ts (6)

5-16: Test Case: accounts for rundown with one event

The test case is well-written and verifies the correct position and width for an event that spans the entire schedule.

18-28: Test Case: accounts for an event that starts halfway and ends at end

The test case correctly verifies the position and width for an event that starts halfway through the schedule and ends at the end.

30-40: Test Case: accounts for an event that starts first and ends halfway

The test case accurately verifies the position and width for an event that starts at the beginning and ends halfway through the schedule.

42-53: Test Case: accounts for an event that is in the middle of the rundown

The test case correctly verifies the position and width for an event that is in the middle of the schedule. The comment explaining the calculation is helpful.

56-60: Test Case: creates an array between the hours given, end excluded

The test case correctly verifies the creation of an array of hourly sections between the given hours, excluding the end hour.

62-65: Test Case: wraps around midnight

The test case accurately verifies the creation of an array of hourly sections that wrap around midnight.

apps/client/src/features/viewers/timeline/TimelinePage.tsx (4)

1-22: Imports and Interface Definition

The imports and interface definition are appropriate and well-organized. The use of types from ontime-types ensures type safety.

24-38: Component Definition and Hooks

The component definition and hooks are well-structured. The use of useMemo for calculating upcoming events is efficient.

39-48: Options and Text Variables

The logic for populating options and defining text variables is clear and concise. The use of localization functions ensures internationalization support.

49-61: Rendering Logic

The rendering logic is well-structured and uses CSS modules for styling. The ViewParamsEditor and Section components are used effectively to render different sections of the timeline.

apps/client/src/features/viewers/timeline/TimelineEntry.tsx (7)

1-21: Imports and Interface Definition

The imports and interface definition are appropriate and well-organized. The use of types ensures type safety.

23-26: Format Options

The format options for time formatting are clearly defined and used consistently.

28-40: Component Definition and Hooks

The component definition and hooks are well-structured. The use of utility functions for formatting time and duration is efficient.

41-67: Rendering Logic

The rendering logic is well-structured and uses CSS modules for styling. The component renders the timeline entry with various styles and content based on the props.

70-73: Imports and Interface Definition

The imports and interface definition are appropriate and well-organized. The use of types ensures type safety.

75-90: Component Definition and Hooks

The component definition and hooks are well-structured. The use of utility functions for getting the status label and localized strings is efficient.

91-92: Rendering Logic

The rendering logic is well-structured and uses CSS modules for styling. The component renders the status text based on the props.

apps/client/src/common/utils/time.ts (2)

12-22: LGTM! The refactor improves readability and maintainability.

The function now uses constants for better clarity in time calculations.

99-120: LGTM! The function is well-implemented and handles edge cases.

The function correctly formats durations and handles negative values gracefully.

apps/client/src/features/viewers/timeline/Timeline.tsx (2)

15-39: LGTM! The function is well-implemented and processes data correctly.

The function handles data retrieval and calculates timeline parameters accurately.

47-114: LGTM! The component is well-implemented and handles different states correctly.

The component calculates element positions and renders timeline entries accurately.

apps/client/src/features/viewers/timeline/timeline.utils.ts (8)

27-29: LGTM! The function correctly calculates the relative position.

The function uses clamping to ensure the value is within the range [0, 100].

34-47: LGTM! The function correctly calculates the absolute position and width.

The function handles the possibility of crossing midnight accurately.

52-55: LGTM! The function correctly calculates the rounded-down hour.

The function uses flooring to get the rounded-down hour.

60-63: LGTM! The function correctly calculates the rounded-up hour.

The function uses ceiling to get the rounded-up hour.

68-74: LGTM! The function correctly converts a time span into an array of hours.

The function iterates over the hours and formats them accurately.

79-94: LGTM! The function correctly extracts timeline sections from a rundown.

The function retrieves events, normalizes the end time, and generates sections accurately.

99-109: LGTM! The function correctly returns a formatted label for a progress status.

The function handles different statuses and formats the time to start accurately.

120-140: LGTM! The function correctly returns upcoming events.

The function retrieves the current, next, and followed-by events accurately.

apps/client/src/AppRouter.tsx (3)

30-30: LGTM! Lazy loading the Timeline component is a good practice.

This approach optimizes performance by loading the component only when needed.

43-43: LGTM! Wrapping Timeline with HOCs ensures it has the necessary presets and data.

This approach maintains consistency with other components in the application.

87-87: LGTM! Adding the /timeline route integrates the new feature into the application's navigation.

This change allows users to access the Timeline view through the specified route.

apps/client/src/common/hooks/useSocket.ts (1)

187-194: LGTM! The useTimelineStatus hook follows a consistent pattern for state management.

This hook enhances the functionality by providing a specific way to access timeline-related state data.

packages/utils/src/rundown-utils/rundownUtils.ts (1)

330-332: LGTM! The getEventWithId function enhances the utility of the rundown data structure.

This function provides a convenient way to retrieve specific events based on their IDs.

apps/client/src/common/utils/styleUtils.ts Show resolved Hide resolved
apps/client/src/features/viewers/timeline/timeline.options.ts Outdated Show resolved Hide resolved
apps/client/src/features/viewers/timeline/timeline-markers/TimelineMarkers.tsx Outdated Show resolved Hide resolved
coderabbitai[bot]
coderabbitai bot reviewed Aug 1, 2024
View reviewed changes
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 89d087c and 6cfdfce.

Files ignored due to path filters (6)
  • apps/cli/package.json is excluded by none and included by none
  • apps/client/package.json is excluded by none and included by none
  • apps/electron/package.json is excluded by none and included by none
  • apps/server/package.json is excluded by none and included by none
  • package.json is excluded by none and included by none
  • packages/utils/index.ts is excluded by none and included by none
Files selected for processing (33)
  • apps/client/src/AppRouter.tsx (3 hunks)
  • apps/client/src/common/components/view-params-editor/ViewParamsEditor.tsx (1 hunks)
  • apps/client/src/common/hooks/useSocket.ts (1 hunks)
  • apps/client/src/common/utils/styleUtils.ts (1 hunks)
  • apps/client/src/common/utils/time.ts (3 hunks)
  • apps/client/src/features/operator/operator-event/OperatorEvent.module.scss (2 hunks)
  • apps/client/src/features/viewers/timeline/Timeline.module.scss (1 hunks)
  • apps/client/src/features/viewers/timeline/Timeline.tsx (1 hunks)
  • apps/client/src/features/viewers/timeline/TimelineEntry.tsx (1 hunks)
  • apps/client/src/features/viewers/timeline/TimelinePage.module.scss (1 hunks)
  • apps/client/src/features/viewers/timeline/TimelinePage.tsx (1 hunks)
  • apps/client/src/features/viewers/timeline/tests/timeline.utils.test.ts (1 hunks)
  • apps/client/src/features/viewers/timeline/timeline-markers/TimelineMarkers.module.scss (1 hunks)
  • apps/client/src/features/viewers/timeline/timeline-markers/TimelineMarkers.tsx (1 hunks)
  • apps/client/src/features/viewers/timeline/timeline-progress-bar/TimelineProgressBar.module.scss (1 hunks)
  • apps/client/src/features/viewers/timeline/timeline-progress-bar/TimelineProgressBar.tsx (1 hunks)
  • apps/client/src/features/viewers/timeline/timeline-section/TimelineSection.module.scss (1 hunks)
  • apps/client/src/features/viewers/timeline/timeline-section/TimelineSection.tsx (1 hunks)
  • apps/client/src/features/viewers/timeline/timeline.options.ts (1 hunks)
  • apps/client/src/features/viewers/timeline/timeline.utils.ts (1 hunks)
  • apps/client/src/theme/_ontimeColours.scss (1 hunks)
  • apps/client/src/theme/_ontimeStyles.scss (1 hunks)
  • apps/client/src/translation/languages/de.ts (1 hunks)
  • apps/client/src/translation/languages/en.ts (1 hunks)
  • apps/client/src/translation/languages/es.ts (1 hunks)
  • apps/client/src/translation/languages/fr.ts (1 hunks)
  • apps/client/src/translation/languages/it.ts (1 hunks)
  • apps/client/src/translation/languages/no.ts (1 hunks)
  • apps/client/src/translation/languages/pl.ts (1 hunks)
  • apps/client/src/translation/languages/pt.ts (1 hunks)
  • apps/client/src/translation/languages/sv.ts (1 hunks)
  • apps/client/src/viewerConfig.ts (1 hunks)
  • packages/utils/src/rundown-utils/rundownUtils.ts (1 hunks)
Files skipped from review due to trivial changes (9)
  • apps/client/src/features/viewers/timeline/Timeline.module.scss
  • apps/client/src/features/viewers/timeline/TimelinePage.module.scss
  • apps/client/src/features/viewers/timeline/timeline-progress-bar/TimelineProgressBar.module.scss
  • apps/client/src/features/viewers/timeline/timeline-section/TimelineSection.module.scss
  • apps/client/src/features/viewers/timeline/timeline.options.ts
  • apps/client/src/theme/_ontimeColours.scss
  • apps/client/src/theme/_ontimeStyles.scss
  • apps/client/src/translation/languages/de.ts
  • apps/client/src/translation/languages/fr.ts
Files skipped from review as they are similar to previous changes (23)
  • apps/client/src/AppRouter.tsx
  • apps/client/src/common/components/view-params-editor/ViewParamsEditor.tsx
  • apps/client/src/common/hooks/useSocket.ts
  • apps/client/src/common/utils/styleUtils.ts
  • apps/client/src/features/operator/operator-event/OperatorEvent.module.scss
  • apps/client/src/features/viewers/timeline/Timeline.tsx
  • apps/client/src/features/viewers/timeline/TimelineEntry.tsx
  • apps/client/src/features/viewers/timeline/TimelinePage.tsx
  • apps/client/src/features/viewers/timeline/tests/timeline.utils.test.ts
  • apps/client/src/features/viewers/timeline/timeline-markers/TimelineMarkers.module.scss
  • apps/client/src/features/viewers/timeline/timeline-markers/TimelineMarkers.tsx
  • apps/client/src/features/viewers/timeline/timeline-progress-bar/TimelineProgressBar.tsx
  • apps/client/src/features/viewers/timeline/timeline-section/TimelineSection.tsx
  • apps/client/src/features/viewers/timeline/timeline.utils.ts
  • apps/client/src/translation/languages/en.ts
  • apps/client/src/translation/languages/es.ts
  • apps/client/src/translation/languages/it.ts
  • apps/client/src/translation/languages/no.ts
  • apps/client/src/translation/languages/pl.ts
  • apps/client/src/translation/languages/pt.ts
  • apps/client/src/translation/languages/sv.ts
  • apps/client/src/viewerConfig.ts
  • packages/utils/src/rundown-utils/rundownUtils.ts
Additional comments not posted (2)
apps/client/src/common/utils/time.ts (2)

12-22: Refactor approved: Improved readability and coding standards.

The refactoring of nowInMillis from an arrow function to a standard function declaration enhances readability and aligns with coding standards. The use of constants for time calculations is correct and improves maintainability.

99-120: New function approved: Robust and clear implementation.

The formatDuration function is well-implemented, providing a clear and maintainable way to format durations. It correctly handles edge cases and uses constants for time calculations.

alex-Arc
alex-Arc approved these changes Aug 1, 2024
View reviewed changes
apps/client/package.json Outdated
@@ -1,6 +1,6 @@
{
"name": "ontime-ui",
"version": "3.4.0",
"version": "3.5.0-beta.1",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this be merged?

apps/client/src/viewerConfig.ts
@@ -3,6 +3,7 @@ export const navigatorConstants = [
{ url: '/clock', label: 'Clock' },
{ url: '/minimal', label: 'Minimal Timer' },
{ url: '/backstage', label: 'Backstage' },
{ url: '/timeline', label: 'Timeline' },
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should maybe also add to the electron menu

coderabbitai[bot]
coderabbitai bot reviewed Aug 4, 2024
View reviewed changes
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Outside diff range, codebase verification and nitpick comments (1)
apps/client/src/features/viewers/timeline/Timeline.tsx (1)

14-32: Add inline comments for clarity.

Adding inline comments to explain the logic in the useTimeline hook would improve readability and maintainability.

function useTimeline(rundown: OntimeEvent[]) {
  // Get the first and last events from the rundown
  const { firstEvent } = getFirstEvent(rundown);
  const { lastEvent } = getLastEvent(rundown);
  const firstStart = firstEvent?.timeStart ?? 0;
  const lastEnd = lastEvent?.timeEnd ?? 0;
  const normalisedLastEnd = lastEnd < firstStart ? lastEnd + dayInMs : lastEnd;

  // Timeline is padded to nearest hours (floor and ceil)
  const startHour = getStartHour(firstStart) * MILLIS_PER_HOUR;
  const endHour = getEndHour(normalisedLastEnd) * MILLIS_PER_HOUR;
  const accumulatedDelay = lastEvent?.delay ?? 0;

  return {
    rundown: rundown,
    startHour,
    endHour,
    accumulatedDelay,
  };
}
Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 6cfdfce and d9f9f26.

Files ignored due to path filters (1)
  • apps/electron/src/menu/applicationMenu.js is excluded by none and included by none
Files selected for processing (6)
  • apps/client/src/features/operator/operator.options.tsx (1 hunks)
  • apps/client/src/features/viewers/timeline/Timeline.tsx (1 hunks)
  • apps/client/src/features/viewers/timeline/TimelinePage.tsx (1 hunks)
  • apps/client/src/features/viewers/timeline/timeline-markers/TimelineMarkers.tsx (1 hunks)
  • apps/client/src/features/viewers/timeline/timeline.options.ts (1 hunks)
  • apps/client/src/features/viewers/timeline/timeline.utils.ts (1 hunks)
Files skipped from review due to trivial changes (1)
  • apps/client/src/features/operator/operator.options.tsx
Files skipped from review as they are similar to previous changes (4)
  • apps/client/src/features/viewers/timeline/TimelinePage.tsx
  • apps/client/src/features/viewers/timeline/timeline-markers/TimelineMarkers.tsx
  • apps/client/src/features/viewers/timeline/timeline.options.ts
  • apps/client/src/features/viewers/timeline/timeline.utils.ts
Additional context used
GitHub Check: unit-test
apps/client/src/features/viewers/timeline/Timeline.tsx

[failure] 45-45:
'searchParams' is assigned a value but never used. Allowed unused elements of array destructuring patterns must match /^_/u

Additional comments not posted (2)
apps/client/src/features/viewers/timeline/Timeline.tsx (2)

34-37: LGTM!

The TimelineProps interface is well-defined.

41-109: LGTM!

The Timeline component is well-implemented and follows best practices.

Tools
GitHub Check: unit-test

[failure] 45-45:
'searchParams' is assigned a value but never used. Allowed unused elements of array destructuring patterns must match /^_/u

apps/client/src/features/viewers/timeline/Timeline.tsx Outdated
Comment on lines 1 to 11
import { memo } from 'react';
import { useSearchParams } from 'react-router-dom';
import { useViewportSize } from '@mantine/hooks';
import { isOntimeEvent, MaybeNumber, OntimeEvent } from 'ontime-types';
import { dayInMs, getFirstEvent, getLastEvent, MILLIS_PER_HOUR } from 'ontime-utils';

import TimelineMarkers from './timeline-markers/TimelineMarkers';
import ProgressBar from './timeline-progress-bar/TimelineProgressBar';
import { getElementPosition, getEndHour, getStartHour } from './timeline.utils';
import { ProgressStatus, TimelineEntry } from './TimelineEntry';

import style from './Timeline.module.scss';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove unused variable searchParams.

The searchParams variable is assigned but never used. This can be removed to clean up the code.

- import { useSearchParams } from 'react-router-dom';
- const [searchParams] = useSearchParams();
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
import { memo } from 'react';
import { useSearchParams } from 'react-router-dom';
import { useViewportSize } from '@mantine/hooks';
import { isOntimeEvent, MaybeNumber, OntimeEvent } from 'ontime-types';
import { dayInMs, getFirstEvent, getLastEvent, MILLIS_PER_HOUR } from 'ontime-utils';
import TimelineMarkers from './timeline-markers/TimelineMarkers';
import ProgressBar from './timeline-progress-bar/TimelineProgressBar';
import { getElementPosition, getEndHour, getStartHour } from './timeline.utils';
import { ProgressStatus, TimelineEntry } from './TimelineEntry';
import style from './Timeline.module.scss';
import { memo } from 'react';
- import { useSearchParams } from 'react-router-dom';
import { useViewportSize } from '@mantine/hooks';
import { isOntimeEvent, MaybeNumber, OntimeEvent } from 'ontime-types';
import { dayInMs, getFirstEvent, getLastEvent, MILLIS_PER_HOUR } from 'ontime-utils';
import TimelineMarkers from './timeline-markers/TimelineMarkers';
import ProgressBar from './timeline-progress-bar/TimelineProgressBar';
import { getElementPosition, getEndHour, getStartHour } from './timeline.utils';
import { ProgressStatus, TimelineEntry } from './TimelineEntry';
import style from './Timeline.module.scss';
- const [searchParams] = useSearchParams();

coderabbitai[bot]
coderabbitai bot reviewed Aug 4, 2024
View reviewed changes
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between d9f9f26 and a503607.

Files ignored due to path filters (7)
  • apps/cli/package.json is excluded by none and included by none
  • apps/client/package.json is excluded by none and included by none
  • apps/electron/package.json is excluded by none and included by none
  • apps/electron/src/menu/applicationMenu.js is excluded by none and included by none
  • apps/server/package.json is excluded by none and included by none
  • package.json is excluded by none and included by none
  • packages/utils/index.ts is excluded by none and included by none
Files selected for processing (34)
  • apps/client/src/AppRouter.tsx (3 hunks)
  • apps/client/src/common/components/view-params-editor/ViewParamsEditor.tsx (1 hunks)
  • apps/client/src/common/hooks/useSocket.ts (1 hunks)
  • apps/client/src/common/utils/styleUtils.ts (1 hunks)
  • apps/client/src/common/utils/time.ts (3 hunks)
  • apps/client/src/features/operator/operator-event/OperatorEvent.module.scss (2 hunks)
  • apps/client/src/features/operator/operator.options.tsx (1 hunks)
  • apps/client/src/features/viewers/timeline/Timeline.module.scss (1 hunks)
  • apps/client/src/features/viewers/timeline/Timeline.tsx (1 hunks)
  • apps/client/src/features/viewers/timeline/TimelineEntry.tsx (1 hunks)
  • apps/client/src/features/viewers/timeline/TimelinePage.module.scss (1 hunks)
  • apps/client/src/features/viewers/timeline/TimelinePage.tsx (1 hunks)
  • apps/client/src/features/viewers/timeline/tests/timeline.utils.test.ts (1 hunks)
  • apps/client/src/features/viewers/timeline/timeline-markers/TimelineMarkers.module.scss (1 hunks)
  • apps/client/src/features/viewers/timeline/timeline-markers/TimelineMarkers.tsx (1 hunks)
  • apps/client/src/features/viewers/timeline/timeline-progress-bar/TimelineProgressBar.module.scss (1 hunks)
  • apps/client/src/features/viewers/timeline/timeline-progress-bar/TimelineProgressBar.tsx (1 hunks)
  • apps/client/src/features/viewers/timeline/timeline-section/TimelineSection.module.scss (1 hunks)
  • apps/client/src/features/viewers/timeline/timeline-section/TimelineSection.tsx (1 hunks)
  • apps/client/src/features/viewers/timeline/timeline.options.ts (1 hunks)
  • apps/client/src/features/viewers/timeline/timeline.utils.ts (1 hunks)
  • apps/client/src/theme/_ontimeColours.scss (1 hunks)
  • apps/client/src/theme/_ontimeStyles.scss (1 hunks)
  • apps/client/src/translation/languages/de.ts (1 hunks)
  • apps/client/src/translation/languages/en.ts (1 hunks)
  • apps/client/src/translation/languages/es.ts (1 hunks)
  • apps/client/src/translation/languages/fr.ts (1 hunks)
  • apps/client/src/translation/languages/it.ts (1 hunks)
  • apps/client/src/translation/languages/no.ts (1 hunks)
  • apps/client/src/translation/languages/pl.ts (1 hunks)
  • apps/client/src/translation/languages/pt.ts (1 hunks)
  • apps/client/src/translation/languages/sv.ts (1 hunks)
  • apps/client/src/viewerConfig.ts (1 hunks)
  • packages/utils/src/rundown-utils/rundownUtils.ts (1 hunks)
Files skipped from review due to trivial changes (10)
  • apps/client/src/common/components/view-params-editor/ViewParamsEditor.tsx
  • apps/client/src/features/operator/operator-event/OperatorEvent.module.scss
  • apps/client/src/features/viewers/timeline/TimelinePage.module.scss
  • apps/client/src/features/viewers/timeline/timeline-progress-bar/TimelineProgressBar.module.scss
  • apps/client/src/features/viewers/timeline/timeline-section/TimelineSection.module.scss
  • apps/client/src/features/viewers/timeline/timeline.options.ts
  • apps/client/src/theme/_ontimeColours.scss
  • apps/client/src/theme/_ontimeStyles.scss
  • apps/client/src/translation/languages/fr.ts
  • apps/client/src/viewerConfig.ts
Files skipped from review as they are similar to previous changes (22)
  • apps/client/src/AppRouter.tsx
  • apps/client/src/common/utils/styleUtils.ts
  • apps/client/src/common/utils/time.ts
  • apps/client/src/features/operator/operator.options.tsx
  • apps/client/src/features/viewers/timeline/Timeline.module.scss
  • apps/client/src/features/viewers/timeline/TimelineEntry.tsx
  • apps/client/src/features/viewers/timeline/TimelinePage.tsx
  • apps/client/src/features/viewers/timeline/tests/timeline.utils.test.ts
  • apps/client/src/features/viewers/timeline/timeline-markers/TimelineMarkers.module.scss
  • apps/client/src/features/viewers/timeline/timeline-markers/TimelineMarkers.tsx
  • apps/client/src/features/viewers/timeline/timeline-progress-bar/TimelineProgressBar.tsx
  • apps/client/src/features/viewers/timeline/timeline-section/TimelineSection.tsx
  • apps/client/src/features/viewers/timeline/timeline.utils.ts
  • apps/client/src/translation/languages/de.ts
  • apps/client/src/translation/languages/en.ts
  • apps/client/src/translation/languages/es.ts
  • apps/client/src/translation/languages/it.ts
  • apps/client/src/translation/languages/no.ts
  • apps/client/src/translation/languages/pl.ts
  • apps/client/src/translation/languages/pt.ts
  • apps/client/src/translation/languages/sv.ts
  • packages/utils/src/rundown-utils/rundownUtils.ts
Additional context used
GitHub Check: unit-test
apps/client/src/features/viewers/timeline/Timeline.tsx

[failure] 45-45:
'searchParams' is assigned a value but never used. Allowed unused elements of array destructuring patterns must match /^_/u

Additional comments not posted (5)
apps/client/src/features/viewers/timeline/Timeline.tsx (4)

14-32: LGTM!

The useTimeline hook is well-structured and correctly calculates the timeline-related data.

41-108: LGTM!

The Timeline component is well-structured and correctly renders the timeline view.

Tools
GitHub Check: unit-test

[failure] 45-45:
'searchParams' is assigned a value but never used. Allowed unused elements of array destructuring patterns must match /^_/u

7-12: LGTM!

The internal functions for rendering timeline markers, progress bars, and timeline entries are well-structured and correctly render the necessary components.

2-2: Remove unused variable searchParams.

The searchParams variable is assigned but never used. This can be removed to clean up the code.

- import { useSearchParams } from 'react-router-dom';

Likely invalid or redundant comment.

apps/client/src/common/hooks/useSocket.ts (1)

188-195: LGTM!

The useTimelineStatus function is well-structured and follows a consistent approach to state management within the module.

apps/client/src/features/viewers/timeline/Timeline.tsx Outdated
Comment on lines 41 to 44
function Timeline(props: TimelineProps) {
const { selectedEventId, rundown: baseRundown } = props;
const { width: screenWidth } = useViewportSize();
const timelineData = useTimeline(baseRundown);
const [searchParams] = useSearchParams();

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove unused variable searchParams.

The searchParams variable is assigned but never used. This can be removed to clean up the code.

-  const [searchParams] = useSearchParams();
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
function Timeline(props: TimelineProps) {
const { selectedEventId, rundown: baseRundown } = props;
const { width: screenWidth } = useViewportSize();
const timelineData = useTimeline(baseRundown);
const [searchParams] = useSearchParams();
function Timeline(props: TimelineProps) {
const { selectedEventId, rundown: baseRundown } = props;
const { width: screenWidth } = useViewportSize();
const timelineData = useTimeline(baseRundown);
Tools
GitHub Check: unit-test

[failure] 45-45:
'searchParams' is assigned a value but never used. Allowed unused elements of array destructuring patterns must match /^_/u

@cpvalente cpvalente merged commit 3012d73 into master Aug 4, 2024
3 checks passed
@cpvalente cpvalente deleted the timeline-view branch August 4, 2024 19:32
@coderabbitai coderabbitai bot mentioned this pull request Sep 18, 2024
Merged
This was referenced Oct 3, 2024
Open
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@coderabbitai coderabbitai[bot] coderabbitai[bot] left review comments

@alex-Arc alex-Arc alex-Arc approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@cpvalente @alex-Arc