feat: add video preview modal - #1429
Conversation
WalkthroughThis change introduces a video preview modal feature for task logs. It adds state management for video modal visibility and URL, updates the modal component to support video content, modifies table column rendering to trigger the modal, and ensures the relevant props and handlers are passed through the component hierarchy. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant TaskLogsTable
participant TaskLogsColumnDefs
participant useTaskLogsData
participant ContentModal
User->>TaskLogsTable: Clicks video link in task log row
TaskLogsTable->>TaskLogsColumnDefs: Renders columns with openVideoModal
TaskLogsColumnDefs->>useTaskLogsData: Calls openVideoModal(url)
useTaskLogsData->>TaskLogsTable: Updates isVideoModalOpen, videoUrl
TaskLogsTable->>ContentModal: Renders video modal with videoUrl and isVideo=true
User->>ContentModal: Views video in modal
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15 minutes Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
web/src/components/table/task-logs/modals/ContentModal.jsx (1)
38-42: Consider removing autoplay and adding error handling for better UX.The conditional rendering logic is correct, but there are a few improvements to consider:
- Autoplay concerns: The
autoPlayattribute may cause accessibility issues and unexpected behavior- Missing error handling: No fallback for video loading failures
- Missing accessibility: Video element lacks proper ARIA labels
Apply this diff to improve the video implementation:
- {isVideo ? ( - <video src={modalContent} controls style={{ width: '100%' }} autoPlay /> - ) : ( - <p style={{ whiteSpace: 'pre-line' }}>{modalContent}</p> - )} + {isVideo ? ( + <video + src={modalContent} + controls + style={{ width: '100%' }} + aria-label="Task video preview" + onError={(e) => console.error('Video loading failed:', e)} + > + Your browser does not support the video tag. + </video> + ) : ( + <p style={{ whiteSpace: 'pre-line' }}>{modalContent}</p> + )}web/src/components/table/task-logs/TaskLogsColumnDefs.js (1)
346-355: Consider improving the anchor implementation for better accessibility.The video modal trigger works correctly, but there are some improvements to consider:
- Semantic HTML: Using
href="#"is not ideal for buttons that trigger actions- Accessibility: A button element would be more semantically correct
Apply this diff to improve the implementation:
- <a - href="#" - onClick={e => { - e.preventDefault(); - openVideoModal(text); - }} - > - {t('点击预览视频')} - </a> + <button + type="button" + onClick={() => openVideoModal(text)} + style={{ + background: 'none', + border: 'none', + color: 'var(--semi-color-primary)', + cursor: 'pointer', + textDecoration: 'underline' + }} + > + {t('点击预览视频')} + </button>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
web/src/components/table/task-logs/TaskLogsColumnDefs.js(2 hunks)web/src/components/table/task-logs/TaskLogsTable.jsx(2 hunks)web/src/components/table/task-logs/index.jsx(1 hunks)web/src/components/table/task-logs/modals/ContentModal.jsx(2 hunks)web/src/hooks/task-logs/useTaskLogsData.js(4 hunks)
🔇 Additional comments (8)
web/src/components/table/task-logs/modals/ContentModal.jsx (1)
27-27: LGTM: Clean prop addition for video functionality.The new
isVideoprop is well-integrated into the component signature.web/src/components/table/task-logs/index.jsx (2)
40-40: Good: Explicit prop makes intent clear.Adding
isVideo={false}explicitly improves code readability and makes the modal's purpose clear.
41-47: Well-structured video modal implementation.The dedicated video modal instance with separate state management is a clean approach that maintains separation of concerns.
web/src/components/table/task-logs/TaskLogsTable.jsx (1)
42-42: Clean integration of video modal functionality.The
openVideoModalprop is correctly destructured, passed togetTaskLogsColumns, and included in theuseMemodependency array. This follows React best practices for prop passing and memoization.Also applies to: 55-55, 63-63
web/src/components/table/task-logs/TaskLogsColumnDefs.js (1)
214-214: LGTM: Parameter correctly added.The
openVideoModalparameter is properly added to the function signature.web/src/hooks/task-logs/useTaskLogsData.js (3)
66-68: Clean state management for video modal.The video modal state variables follow the existing pattern in the hook and are properly initialized.
250-254: Well-implemented modal handler function.The
openVideoModalfunction correctly sets both the video URL and modal visibility state in a single operation, following the same pattern as the existingopenContentModalfunction.
277-280: Proper state exposure in hook return.The new video modal states and handler are correctly added to the hook's return object, making them available to consuming components.
Also applies to: 308-308
…-modal feat: add video preview modal
2 participants
增加视频预览模态窗
Summary by CodeRabbit
New Features
Enhancements