feat-remote-control-ui-server-integration #652
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Note
|
| Cohort / File(s) | Summary |
|---|---|
Remote Control Web Application apps/remote-control-web/src/app/app.*, apps/remote-control-web/src/index.html, apps/remote-control-web/src/main.ts, apps/remote-control-web/src/styles.scss |
New Angular standalone application entry point with component root, global styling, configuration, and HTML bootstrap. Configures i18n via TranslateService. |
Remote Control Web Configuration apps/remote-control-web/project.json, apps/remote-control-web/tsconfig*, apps/remote-control-web/eslint.config.mjs, apps/remote-control-web/jest.config.ts |
Nx project configuration, TypeScript build/test/lib configs, ESLint rules, and Jest setup for the remote-control-web application. |
Shared Remote Control Library libs/ui/remote-control/src/lib/remote-control/remote-control.*, libs/ui/remote-control/src/index.ts, libs/ui/remote-control/project.json |
New Angular library component with HTML template (touchpad UI with up/down buttons, status indicator), SCSS styling (full-screen dark theme, animations, responsive layout), TypeScript service for HTTP API calls (channelUp/channelDown), and component orchestrating loading/error states. |
Remote Control Library Configuration libs/ui/remote-control/tsconfig*, libs/ui/remote-control/eslint.config.mjs, libs/ui/remote-control/jest.config.ts, libs/ui/remote-control/src/test-setup.ts, libs/ui/remote-control/README.md |
TypeScript configs (lib/spec), ESLint/Jest setup, test environment initialization, and documentation for the remote-control library. |
Electron Backend HTTP Server apps/electron-backend/src/app/server/http-server.ts |
New HTTP server module serving remote-control-web static assets, routing /api/remote-control/* endpoints to registered handlers, with start/stop lifecycle and configurable port (default 8765). Supports development and production asset paths. |
Electron Backend Remote Control Events apps/electron-backend/src/app/events/remote-control.events.ts, apps/electron-backend/src/app/events/settings.events.ts |
New event handler bootstrapping HTTP endpoints (/api/remote-control/channel/up |
Electron Backend Integration apps/electron-backend/src/app/api/main.preload.ts, apps/electron-backend/src/main.ts, apps/electron-backend/project.json |
Adds onChannelChange IPC listener to preload bridge, bootstraps RemoteControlEvents in app startup, updates build dependencies to include remote-control-web. |
Web App Video Player Integration apps/web/src/app/home/video-player/video-player.component.ts |
Listens to Electron onChannelChange callback; handles remote direction events (up/down) by computing next active channel with wraparound and dispatching to store. Refactors overlay ref injection to use public COMPONENT_OVERLAY_REF token. |
Overlay Reference Token libs/ui/components/src/lib/multi-epg/overlay-ref.token.ts, libs/ui/components/src/lib/multi-epg/multi-epg-container.component.ts, libs/ui/components/src/index.ts |
Extracts InjectionToken for OverlayRef into public shareable token; updates multi-epg container to use centralized token; exports token from components library index. |
Channel Selection Refactor libs/ui/components/src/lib/channel-list-container/channel-list-container.component.* |
Replaces id-based channel selection (selected property) with URL-based active channel via activeChannel signal (selectActive from store); updates bindings in all channel lists. |
Settings & Localization apps/web/src/app/settings/settings.component.*, apps/web/src/app/services/settings-store.service.ts, apps/web/src/assets/i18n/en.json |
Changes remote control default port from 3000 to 8765; limits remote control UI to desktop only (removes PWA condition); updates i18n descriptions and adds SUBTITLE for remote control block. |
Type Definitions & Configuration global.d.ts, tsconfig.base.json |
Adds onChannelChange optional method to Window.electron interface; adds "remote-control" path alias to tsconfig base. |
Sequence Diagram(s)
sequenceDiagram
participant User as Remote Control UI<br/>(Browser)
participant HTTP as HTTP Server<br/>(Electron)
participant Main as Main Process<br/>(Electron)
participant Renderer as Renderer Process<br/>(Electron)
participant Store as App Store
participant Player as Video Player<br/>(Angular Web)
User->>HTTP: POST /api/remote-control/channel/up
HTTP->>Main: channelUp handler invoked
Main->>Main: Dispatch CHANNEL_CHANGE message
Main->>Renderer: webContents.send('CHANNEL_CHANGE',<br/>{direction: 'up'})
Renderer->>Renderer: onChannelChange callback fired
Renderer->>Store: Compute next channel,<br/>dispatch setActiveChannel
Store->>Player: activeChannel signal updates
Player->>Player: Render new channel
Player-->>User: UI reflects channel change
sequenceDiagram
participant App as Remote Control Web App
participant Service as RemoteControlService
participant Backend as HTTP Backend<br/>(/api/remote-control/*)
participant HTTP as HTTP Server
participant Events as RemoteControlEvents
participant IPC as IPC Bridge
participant Video as Video Player
App->>Service: changeChannelUp() called
Service->>Backend: POST /api/remote-control/channel/up
Backend->>HTTP: Routes to registered handler
HTTP->>Events: Invokes channelUp handler
Events->>Events: Log action
Events->>IPC: Send CHANNEL_CHANGE via IPC
IPC->>Video: Listener fires with {direction: 'up'}
Video->>Video: handleRemoteChannelChange('up')
Note over Video: Compute next channel<br/>& dispatch setActiveChannel
Estimated code review effort
🎯 4 (Complex) | ⏱️ ~60 minutes
- Architectural integration complexity: The PR wires together multiple systems (HTTP server, IPC, event handlers, store updates, UI components) requiring end-to-end flow verification.
- Security considerations: HTTP server implementation, request validation, and static file serving with path normalization need careful review.
- Integration points requiring attention:
- HTTP server start/stop lifecycle and port management (dynamic updates via settings)
- IPC message flow and callback registration in preload bridge
- Video player's remote channel change handler logic (wraparound channel selection)
- Store interaction and activeChannel signal refactor in channel list components
- Type safety of onChannelChange callback in global.d.ts and preload
- New module quality: Remote control service HTTP calls, component loading/error state handling, and template bindings need verification.
- Heterogeneity: Mix of new files (web app, library, HTTP server), refactors (overlay token extraction, channel selection), and integrations (settings, video player) increases cognitive load.
Possibly related PRs
- PR
#648: Modifies video-player component, COMPONENT_OVERLAY_REF token integration, and IPC preload API changes that overlap with this PR's integration points. - PR
#638: Updates Electron preload API and event wiring patterns used for the new IPC-based remote control channel change listener.
Poem
🐰 A touchpad springs to life so bright,
Channels dance with up and down delight,
From web to Electron, messages fly,
Remote control brings magic nigh—
One HTTP, one IPC, one view unified! 📱✨
Pre-merge checks and finishing touches
❌ Failed checks (1 warning)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. | You can run @coderabbitai generate docstrings to improve docstring coverage. |
✅ Passed checks (2 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | ✅ Passed | The title 'feat-remote-control-ui-server-integration' clearly relates to the main feature changes: adding remote control UI, integrating with the HTTP server, and connecting them to the backend. |
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.
Comment @coderabbitai help to get the list of available commands and usage tips.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (17)
libs/ui/remote-control/README.md (1)
1-7: LGTM - Consider expanding documentation in future.The README provides the essential information for running tests. As the library evolves, consider adding sections covering the library's purpose, usage examples, and integration guide.
libs/ui/remote-control/src/lib/remote-control/remote-control.service.ts (2)
13-13: Consider making the API base URL configurable.The hardcoded relative URL assumes the remote control service runs on the same origin. For the current Electron use case this is correct, but consider exposing configuration if cross-origin scenarios are anticipated.
20-33: Consider adding error handling for HTTP failures.Both methods use
firstValueFromwithout catching potential HTTP errors. While it's acceptable to let consumers handle errors, consider adding at least basic error logging or wrapping errors with context for better debugging.Example:
async channelUp(): Promise<void> { - await firstValueFrom( - this.http.post<void>(`${this.apiBaseUrl}/channel/up`, {}) - ); + try { + await firstValueFrom( + this.http.post<void>(`${this.apiBaseUrl}/channel/up`, {}) + ); + } catch (error) { + console.error('Failed to send channel up command:', error); + throw error; + } }apps/electron-backend/src/app/events/settings.events.ts (2)
22-37: Good settings propagation logic, but consider adding error handling.The implementation correctly handles defaults and persists settings before applying them. However,
httpServer.updateSettingsmight throw if the port is invalid or already in use.Consider wrapping the call in try-catch:
// Update HTTP server - httpServer.updateSettings(enabled, port); + try { + httpServer.updateSettings(enabled, port); + } catch (error) { + console.error('Failed to update remote control server:', error); + // Optionally: revert store values or notify renderer + }
25-25: Consider validating the port number.While the default port 8765 is valid, user-provided ports should be validated to ensure they're in the valid range (1-65535) and ideally check if the port is available.
Example validation:
const port = arg.remoteControlPort ?? store.get('remoteControlPort', 8765); if (port < 1 || port > 65535) { console.error(`Invalid port: ${port}`); return; // or throw an error }apps/remote-control-web/src/app/app.ts (1)
14-17: Consider simplifying i18n initialization.Both
setDefaultLang('en')anduse('en')are called. Since the language is the same, you could use onlysetDefaultLang('en')which is typically sufficient for initial setup.ngOnInit() { - // Set default language this.translate.setDefaultLang('en'); - this.translate.use('en'); }apps/remote-control-web/project.json (1)
33-43: Consider adjusting initial bundle budget for remote control UI.The initial bundle budget is set to 500kb warning / 1mb error. For a simple remote control UI, these limits might be generous. Consider tightening them (e.g., 300kb/500kb) to maintain a lean bundle size for this focused application.
libs/ui/remote-control/src/lib/remote-control/remote-control.component.html (1)
5-5: Consider internationalizing hardcoded strings.The template contains hardcoded strings like "IPTVnator", "Switching", and "Ready" that could benefit from i18n support for consistency with the rest of the application.
- <div class="remote__logo">IPTVnator</div> + <div class="remote__logo">{{ 'REMOTE_CONTROL.BRANDING' | translate }}</div><span class="status-text">{{ - isLoading ? 'Switching' : 'Ready' + isLoading ? ('REMOTE_CONTROL.STATUS.SWITCHING' | translate) : ('REMOTE_CONTROL.STATUS.READY' | translate) }}</span>Also applies to: 72-74
apps/web/src/app/home/video-player/video-player.component.ts (1)
172-172: Remove console.log statements for production.Multiple debug console.log statements are present in the remote channel change handler. Consider removing these or replacing with a proper logging service for production code.
Apply this diff to remove console.log statements:
- console.log(`Remote control: changing channel ${direction}`); - // Use combineLatest to get both values and take only the first emission combineLatest([this.channels$, this.activeChannel$]) .pipe( @@ -189,7 +186,6 @@ if (currentIndex === -1) { - console.warn('Current channel not found in channel list'); return; } @@ -206,11 +202,8 @@ // Dispatch action to change channel const nextChannel = channels[nextIndex]; - console.log(`Switching to channel: ${nextChannel.name}`); this.store.dispatch(PlaylistActions.setActiveChannel({ channel: nextChannel })); }, error: (err) => { - console.error('Error changing channel:', err); }Also applies to: 193-193, 209-209, 213-213
libs/ui/remote-control/src/lib/remote-control/remote-control.component.scss (1)
1-7: Consider scopinguser-select: nonemore narrowly.Applying
user-select: noneto all elements (*) prevents text selection throughout the UI, which may impact accessibility. Users may want to copy error messages or status text. Consider applying this property only to interactive elements (buttons, touchpad) rather than globally.Apply this diff to scope user-select more appropriately:
* { margin: 0; padding: 0; box-sizing: border-box; -webkit-tap-highlight-color: transparent; - user-select: none; } + +.touchpad__button, +.touchpad__glass { + user-select: none; +}apps/electron-backend/src/app/events/remote-control.events.ts (2)
30-36: IPC handlers should return a value or not be async.The IPC handlers for
REMOTE_CONTROL_CHANNEL_UPandREMOTE_CONTROL_CHANNEL_DOWNare declared asasyncbut the underlying methods (changeChannelUp/changeChannelDown) returnvoidand don't use async operations. Either remove theasynckeyword or have the methods return a meaningful value.Apply this diff to make the handlers consistent:
- ipcMain.handle('REMOTE_CONTROL_CHANNEL_UP', async () => { - return this.changeChannelUp(); + ipcMain.on('REMOTE_CONTROL_CHANNEL_UP', () => { + this.changeChannelUp(); }); - ipcMain.handle('REMOTE_CONTROL_CHANNEL_DOWN', async () => { - return this.changeChannelDown(); + ipcMain.on('REMOTE_CONTROL_CHANNEL_DOWN', () => { + this.changeChannelDown(); });
96-104: Consider handling multiple windows more robustly.The code assumes the first window in
BrowserWindow.getAllWindows()is the correct target. In scenarios with multiple windows or if windows are destroyed and recreated, this could send the channel change to the wrong window or miss the target entirely.Consider one of these approaches:
- Store a reference to the main window when it's created
- Add window identification logic to find the video player window
- Broadcast to all windows and let each decide if it should handle the event
private sendChannelChangeToRenderer(direction: 'up' | 'down'): void { const windows = BrowserWindow.getAllWindows(); if (windows.length === 0) { console.warn('No browser windows found to send channel change'); return; } // Broadcast to all windows windows.forEach(window => { window.webContents.send('CHANNEL_CHANGE', { direction }); }); console.log(`Sent CHANNEL_CHANGE ${direction} to ${windows.length} window(s)`); }apps/electron-backend/src/app/server/http-server.ts (4)
11-12: Remove redundant type annotations.The type annotations for
portandisEnabledare trivially inferred from their literal values.Apply this diff as suggested by ESLint:
- private port: number = 8765; - private isEnabled: boolean = false; + private port = 8765; + private isEnabled = false;
62-74: Add request timeout configuration to prevent resource exhaustion.The HTTP server is created without timeout configuration, which could lead to resource exhaustion if clients keep connections open indefinitely. Consider adding timeouts for headers, requests, and keep-alive.
Apply this diff to add timeout configuration:
this.server = http.createServer((req, res) => { this.handleRequest(req, res); }); + + // Configure timeouts to prevent resource exhaustion + this.server.setTimeout(30000); // 30 seconds + this.server.headersTimeout = 10000; // 10 seconds + this.server.requestTimeout = 30000; // 30 seconds this.server.listen(this.port, () => {
121-142: Add CORS headers for local network access.The HTTP server doesn't set CORS headers, which will prevent the remote control web app from being accessed from browsers on other devices in the local network. Consider adding appropriate CORS headers to allow local network access.
Apply this diff to add CORS support:
private handleRequest( req: http.IncomingMessage, res: http.ServerResponse ): void { const url = req.url || '/'; + + // Set CORS headers for local network access + res.setHeader('Access-Control-Allow-Origin', '*'); + res.setHeader('Access-Control-Allow-Methods', 'GET, POST, OPTIONS'); + res.setHeader('Access-Control-Allow-Headers', 'Content-Type'); + + // Handle preflight requests + if (req.method === 'OPTIONS') { + res.writeHead(204); + res.end(); + return; + } // Handle API requests
156-173: Add error logging for file serving failures.File read errors are handled, but only ENOENT errors are logged implicitly. Other file system errors (permissions, I/O errors) fail silently, making debugging difficult.
Apply this diff to add error logging:
fs.readFile(fullPath, (err, data) => { if (err) { + console.error(`[HTTP Server] Error reading file ${fullPath}:`, err); + // If file not found, try serving index.html (for Angular routing) if (err.code === 'ENOENT' && filePath !== '/index.html') {libs/ui/remote-control/src/lib/remote-control/remote-control.component.ts (1)
17-28: Consider more specific error messages and internationalization.The error messages are currently hardcoded generic strings ('Failed to change channel up/down'). Consider:
- Using more specific error messages based on the error type (network error, server error, etc.)
- Internationalizing error messages for multi-language support
Example approach with more specific errors:
async changeChannelUp(): Promise<void> { this.isLoading = true; this.error = null; try { await this.remoteControlService.channelUp(); } catch (err) { if (err instanceof TypeError && err.message.includes('fetch')) { this.error = 'Network error: Cannot connect to server'; } else if (err instanceof Error && err.message.includes('404')) { this.error = 'Service not available'; } else { this.error = 'Failed to change channel up'; } console.error(err); } finally { this.isLoading = false; } }Also applies to: 30-41
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
apps/remote-control-web/public/favicon.icois excluded by!**/*.ico
📒 Files selected for processing (46)
apps/electron-backend/project.json(2 hunks)apps/electron-backend/src/app/api/main.preload.ts(1 hunks)apps/electron-backend/src/app/events/remote-control.events.ts(1 hunks)apps/electron-backend/src/app/events/settings.events.ts(2 hunks)apps/electron-backend/src/app/server/http-server.ts(1 hunks)apps/electron-backend/src/main.ts(2 hunks)apps/remote-control-web/README.md(1 hunks)apps/remote-control-web/eslint.config.mjs(1 hunks)apps/remote-control-web/jest.config.ts(1 hunks)apps/remote-control-web/project.json(1 hunks)apps/remote-control-web/src/app/app.config.ts(1 hunks)apps/remote-control-web/src/app/app.html(1 hunks)apps/remote-control-web/src/app/app.scss(1 hunks)apps/remote-control-web/src/app/app.ts(1 hunks)apps/remote-control-web/src/index.html(1 hunks)apps/remote-control-web/src/main.ts(1 hunks)apps/remote-control-web/src/styles.scss(1 hunks)apps/remote-control-web/src/test-setup.ts(1 hunks)apps/remote-control-web/tsconfig.app.json(1 hunks)apps/remote-control-web/tsconfig.json(1 hunks)apps/remote-control-web/tsconfig.spec.json(1 hunks)apps/web/src/app/home/video-player/video-player.component.ts(5 hunks)apps/web/src/app/services/settings-store.service.ts(1 hunks)apps/web/src/app/settings/settings.component.html(1 hunks)apps/web/src/app/settings/settings.component.ts(1 hunks)apps/web/src/assets/i18n/en.json(2 hunks)global.d.ts(1 hunks)libs/ui/components/src/index.ts(1 hunks)libs/ui/components/src/lib/channel-list-container/channel-list-container.component.html(3 hunks)libs/ui/components/src/lib/channel-list-container/channel-list-container.component.ts(3 hunks)libs/ui/components/src/lib/multi-epg/multi-epg-container.component.ts(2 hunks)libs/ui/components/src/lib/multi-epg/overlay-ref.token.ts(1 hunks)libs/ui/remote-control/README.md(1 hunks)libs/ui/remote-control/eslint.config.mjs(1 hunks)libs/ui/remote-control/jest.config.ts(1 hunks)libs/ui/remote-control/project.json(1 hunks)libs/ui/remote-control/src/index.ts(1 hunks)libs/ui/remote-control/src/lib/remote-control/remote-control.component.html(1 hunks)libs/ui/remote-control/src/lib/remote-control/remote-control.component.scss(1 hunks)libs/ui/remote-control/src/lib/remote-control/remote-control.component.ts(1 hunks)libs/ui/remote-control/src/lib/remote-control/remote-control.service.ts(1 hunks)libs/ui/remote-control/src/test-setup.ts(1 hunks)libs/ui/remote-control/tsconfig.json(1 hunks)libs/ui/remote-control/tsconfig.lib.json(1 hunks)libs/ui/remote-control/tsconfig.spec.json(1 hunks)tsconfig.base.json(1 hunks)
🧰 Additional context used
🪛 ESLint
apps/electron-backend/src/app/server/http-server.ts
[error] 11-11: Type number trivially inferred from a number literal, remove type annotation.
(@typescript-eslint/no-inferrable-types)
[error] 12-12: Type boolean trivially inferred from a boolean literal, remove type annotation.
(@typescript-eslint/no-inferrable-types)
🪛 markdownlint-cli2 (0.18.1)
apps/remote-control-web/README.md
21-21: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
100-100: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
118-118: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (52)
libs/ui/components/src/index.ts (1)
12-12: LGTM - Clean export of shared overlay token.The new export makes the COMPONENT_OVERLAY_REF injection token publicly accessible, enabling shared overlay management across components.
apps/remote-control-web/src/index.html (1)
1-13: LGTM - Standard Angular entry point.The HTML structure is correct with all necessary meta tags, base href, and the app-root element. The file follows Angular best practices for standalone applications.
tsconfig.base.json (1)
25-26: LGTM - Path alias correctly configured.The new "remote-control" path alias is properly configured and follows the existing pattern. This enables clean imports throughout the application.
libs/ui/remote-control/project.json (1)
1-21: LGTM - Standard Nx project configuration.The project configuration correctly defines test and lint targets with appropriate executors and paths. The setup aligns with Nx conventions.
apps/web/src/assets/i18n/en.json (2)
170-170: LGTM - Port description updated to match default.The port description correctly reflects the new default port of 8765, aligning with the changes across the codebase.
345-347: LGTM - Improved remote control UI text.The additions and refinements to the remote control strings enhance clarity. The new subtitle provides helpful context, and the updated channel navigation labels ("Channel Up"/"Channel Down") are more intuitive than the previous versions.
apps/remote-control-web/README.md (1)
1-174: Excellent comprehensive documentation.The README is thorough and well-structured, covering architecture, usage, development workflows, security considerations, and future enhancements. This will be very helpful for users and contributors.
apps/remote-control-web/eslint.config.mjs (1)
1-34: LGTM - Standard Angular ESLint configuration.The configuration correctly extends the base config and Nx Angular presets, with appropriate selector rules enforcing the "app" prefix and consistent naming conventions. The setup follows Angular best practices.
libs/ui/remote-control/eslint.config.mjs (1)
1-34: LGTM!The ESLint configuration is appropriate for an Angular library. The use of the
libprefix for component and directive selectors follows Angular conventions for library packages.libs/ui/components/src/lib/channel-list-container/channel-list-container.component.ts (2)
31-35: LGTM!The addition of
selectActiveto the imports correctly supports the new store-backed active channel mechanism.
130-142: LGTM!The
selectChannelmethod correctly dispatches thesetActiveChannelaction to the store and handles EPG fetching appropriately.libs/ui/components/src/lib/channel-list-container/channel-list-container.component.html (2)
45-45: LGTM!The selection binding correctly uses the
activeChannel()signal with proper optional chaining.
139-139: LGTM!The selection binding in the Favorites list correctly uses optional chaining.
apps/remote-control-web/src/app/app.html (1)
1-1: LGTM!The template correctly references the remote control library component.
apps/remote-control-web/src/app/app.scss (1)
1-4: LGTM!The host styles appropriately set up the component for full viewport height display.
apps/web/src/app/services/settings-store.service.ts (1)
20-32: LGTM!The default remote control port change from 3000 to 8765 aligns with the PR objectives and is consistent with the remote control server implementation.
apps/web/src/app/settings/settings.component.html (1)
273-289: LGTM!The remote control settings are correctly gated to desktop-only contexts using the
@if (isDesktop)guard, which aligns with the PR's scope of limiting remote control to Electron builds.apps/remote-control-web/src/main.ts (1)
1-5: LGTM!The bootstrap logic follows the standard Angular standalone application pattern with appropriate error handling.
apps/web/src/app/settings/settings.component.ts (1)
148-148: LGTM! Port default aligns with backend.The default port change to 8765 correctly matches the backend HTTP server default mentioned in the PR objectives, ensuring consistency across the stack.
apps/remote-control-web/src/styles.scss (1)
3-11: LGTM! Appropriate base styles for a remote control UI.The dark theme and fullscreen layout are well-suited for a remote control interface.
apps/remote-control-web/tsconfig.json (1)
1-32: LGTM! Well-configured TypeScript project with strict mode.The configuration follows Angular best practices with strict compiler options and modern module resolution. This will help catch potential issues during development.
apps/remote-control-web/tsconfig.app.json (1)
1-14: LGTM! Standard app configuration.The configuration correctly extends the base config and properly excludes test files from the application build.
libs/ui/remote-control/src/test-setup.ts (1)
1-6: LGTM! Strict test configuration enhances quality.Enabling
errorOnUnknownElementsanderrorOnUnknownPropertieshelps catch template issues early during testing.libs/ui/remote-control/src/index.ts (1)
1-2: LGTM! Clean library exports.The barrel export pattern correctly exposes the component and service for external consumption.
apps/remote-control-web/src/test-setup.ts (1)
1-6: LGTM! Well-configured Angular test setup.The test setup correctly initializes the Angular testing environment with Zone.js support and enables strict validation for unknown elements and properties, which helps catch template issues early.
global.d.ts (1)
159-162: LGTM! Well-typed remote control API.The optional
onChannelChangecallback is appropriately typed with a clear direction union type. Making it optional maintains backward compatibility with existing code.libs/ui/components/src/lib/multi-epg/overlay-ref.token.ts (1)
1-9: LGTM! Excellent refactoring for token reusability.Extracting the injection token into a shared module enables consistent dependency injection across multiple components and prevents duplicate token creation. The descriptive comment clearly documents its purpose.
libs/ui/remote-control/tsconfig.spec.json (1)
1-17: LGTM! Standard Jest test configuration.The TypeScript test configuration follows Angular library conventions with appropriate compiler options for Jest, correct test file patterns, and proper test setup file reference.
libs/ui/components/src/lib/multi-epg/multi-epg-container.component.ts (2)
25-25: LGTM! Improved token management.Importing the shared
COMPONENT_OVERLAY_REFtoken aligns with the refactoring to centralize injection token definitions and improve maintainability across components.
92-92: LGTM! Consistent token usage.Using the shared injection token simplifies the component by removing the local token definition while maintaining type safety and functionality.
apps/remote-control-web/tsconfig.spec.json (1)
1-17: LGTM! Well-structured test configuration.The TypeScript test configuration is properly configured for Jest with Angular, using appropriate compiler options and test file patterns.
libs/ui/remote-control/tsconfig.lib.json (1)
1-17: LGTM! Proper library build configuration.The TypeScript library configuration correctly enables declaration generation, declaration maps, and source inlining while appropriately excluding test files from the library build output.
apps/electron-backend/project.json (1)
10-10: Build dependency addition verified successfully.The remote-control-web project exists and has a complete, valid build configuration using the Angular build executor with proper output paths, entry points, and build options for both production and development environments. The dependency addition to the Electron backend is correct.
libs/ui/remote-control/tsconfig.json (1)
1-30: LGTM!The TypeScript configuration follows Angular best practices with appropriate strict mode settings and project references. The setup aligns well with modern Angular library standards.
apps/remote-control-web/src/app/app.config.ts (2)
12-32: LGTM!The application configuration is well-structured with appropriate providers for i18n, HTTP client, and Angular essentials. The translation loader configuration follows ngx-translate best practices.
1-10: Remove duplicate HttpClient import.
HttpClientis imported twice: once on line 7 viaprovideHttpClientimport and again on line 10. Remove the duplicate import on line 10.import { ApplicationConfig, importProvidersFrom, provideBrowserGlobalErrorListeners, provideZoneChangeDetection, } from '@angular/core'; -import { provideHttpClient } from '@angular/common/http'; +import { provideHttpClient, HttpClient } from '@angular/common/http'; import { TranslateModule, TranslateLoader } from '@ngx-translate/core'; import { TranslateHttpLoader } from '@ngx-translate/http-loader'; -import { HttpClient } from '@angular/common/http';Likely an incorrect or invalid review comment.
apps/electron-backend/src/main.ts (2)
10-10: LGTM!The import of
RemoteControlEventsfollows the existing pattern for event modules in this file.
44-44: LGTM!The bootstrap call for
RemoteControlEventsis correctly positioned after database initialization and alongside other event bootstrapping calls. The placement ensures proper initialization order.apps/remote-control-web/src/app/app.ts (1)
1-18: LGTM!The component follows modern Angular patterns with the inject() API and proper i18n setup. The structure is clean and well-organized.
libs/ui/remote-control/jest.config.ts (1)
1-21: LGTM!The Jest configuration follows Angular testing best practices with jest-preset-angular and includes all necessary transformers and serializers for proper Angular component testing.
apps/remote-control-web/project.json (2)
1-96: LGTM!The Nx project configuration is well-structured with all necessary build, serve, test, and lint targets properly configured. The setup follows Nx and Angular best practices.
23-27: Path configuration verified as intentional and stable.The i18n assets at
apps/web/src/assets/i18nare confirmed to exist (17 language files), and the cross-app sharing in remote-control-web is intentional. Only remote-control-web explicitly references this path for build-time asset copying, while apps/web includes all assets via broader glob. The architecture is sound with no conflicting configurations.libs/ui/remote-control/src/lib/remote-control/remote-control.component.html (2)
19-54: LGTM!The button implementation is well-structured with proper event handlers, disabled states, loading indicators, and accessibility attributes (aria-label). The SVG icons are cleanly embedded.
1-77: LGTM!The template is well-structured with good visual organization, proper state management bindings, and accessibility considerations. The touchpad design with Up/Down buttons and loading states creates an intuitive remote control interface.
apps/web/src/app/home/video-player/video-player.component.ts (3)
125-130: LGTM: Remote control listener setup is correct.The conditional check for desktop environment and proper type-safe callback registration are well implemented.
199-204: Verify direction semantics: "up" = previous, "down" = next.The current implementation maps "up" to the previous channel (decreasing index) and "down" to the next channel (increasing index). This may be counterintuitive in some contexts where "up" might be expected to mean forward/next in a list. Please confirm this is the intended behavior for your remote control UX.
270-270: LGTM: Proper use of public overlay reference token.The migration from a private InjectionToken to the publicly exported COMPONENT_OVERLAY_REF token improves code reusability and maintainability.
libs/ui/remote-control/src/lib/remote-control/remote-control.component.scss (2)
80-93: LGTM: Well-structured glassmorphic design.The backdrop-filter and layered effects create a modern, polished UI. The use of pseudo-elements for reflections and shadows is efficient and maintainable.
392-398: LGTM: Excellent accessibility support for reduced motion.The prefers-reduced-motion media query properly disables animations for users who prefer reduced motion, which is an important accessibility consideration.
apps/remote-control-web/jest.config.ts (1)
1-21: LGTM: Standard Jest configuration.The Jest configuration is properly set up for an Angular application with appropriate transforms, serializers, and coverage settings.
apps/electron-backend/src/app/server/http-server.ts (1)
152-152: LGTM: Proper path traversal protection.The path normalization with regex to strip leading
../sequences effectively prevents directory traversal attacks.libs/ui/remote-control/src/lib/remote-control/remote-control.component.ts (1)
14-15: LGTM: Proper state management for loading and errors.The component correctly manages loading state and error messages, ensuring proper UI feedback during async operations.
| // Remote control channel change listener | ||
| onChannelChange: (callback: (data: { direction: 'up' | 'down' }) => void) => { | ||
| ipcRenderer.on('CHANNEL_CHANGE', (_event, data) => callback(data)); | ||
| }, |
There was a problem hiding this comment.
Add cleanup mechanism for the IPC listener.
The onChannelChange listener registration doesn't provide a way to unsubscribe, which can lead to memory leaks if multiple listeners are registered or if the component using this API is destroyed and recreated.
Consider returning a cleanup function:
- onChannelChange: (callback: (data: { direction: 'up' | 'down' }) => void) => {
- ipcRenderer.on('CHANNEL_CHANGE', (_event, data) => callback(data));
- },
+ onChannelChange: (callback: (data: { direction: 'up' | 'down' }) => void) => {
+ const handler = (_event: any, data: any) => callback(data);
+ ipcRenderer.on('CHANNEL_CHANGE', handler);
+ return () => ipcRenderer.removeListener('CHANNEL_CHANGE', handler);
+ },🤖 Prompt for AI Agents
In apps/electron-backend/src/app/api/main.preload.ts around lines 4 to 7, the
onChannelChange IPC listener is registered but not removed; change its API to
return an unsubscribe/cleanup function that calls ipcRenderer.removeListener (or
ipcRenderer.off) for the 'CHANNEL_CHANGE' event, and ensure the listener
callback is stored as a named function reference so removeListener can
unregister exactly that handler; update callers to call the returned cleanup
when the component unmounts or no longer needs the listener.
| [selected]=" | ||
| selected?.id === channel.id | ||
| activeChannel()?.url === channel.url | ||
| " |
There was a problem hiding this comment.
Add optional chaining for consistency.
Line 93 uses channel.url without optional chaining, while lines 45 and 139 use channel?.url. Although the @for loop context likely ensures channel is defined, using optional chaining consistently improves code safety and maintainability.
Apply this diff:
[selected]="
- activeChannel()?.url === channel.url
+ activeChannel()?.url === channel?.url
"📝 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.
| [selected]=" | |
| selected?.id === channel.id | |
| activeChannel()?.url === channel.url | |
| " | |
| [selected]=" | |
| activeChannel()?.url === channel?.url | |
| " |
🤖 Prompt for AI Agents
In
libs/ui/components/src/lib/channel-list-container/channel-list-container.component.html
around lines 92 to 94, the [selected] binding uses channel.url without optional
chaining; update it to use channel?.url to match the other usages (lines 45 and
139) for consistency and safety — modify the expression so it reads
activeChannel()?.url === channel?.url.
|
|
||
| /** Selected channel */ | ||
| selected!: Channel; | ||
| readonly activeChannel = this.store.selectSignal(selectActive); |
There was a problem hiding this comment.
🧩 Analysis chain
Verify the selection key change from id to url.
The migration from a local selected property to a store-backed activeChannel signal is excellent. However, this change also switches the channel selection comparison key from channel.id to channel.url (as seen in the template bindings). Ensure that:
- Channel URLs are stable and unique identifiers
- All channel objects have a
urlproperty - This change is intentional and won't cause selection mismatches
Run the following script to verify channel URL usage patterns:
🏁 Script executed:
#!/bin/bash
# Description: Verify channel URL usage and uniqueness assumptions
# Check for Channel interface definition to confirm url property
ast-grep --pattern $'interface Channel {
$$$
url: $$$
$$$
}'
# Search for channel selection and comparison patterns
rg -nP --type=ts -C3 'channel\.(id|url)' -g '!**/node_modules/**' -g '!**/dist/**'Length of output: 10251
🏁 Script executed:
#!/bin/bash
# Find selectActive selector definition
rg -nP --type=ts 'selectActive|activeChannel' -C2 -g '!**/node_modules/**' -g '!**/dist/**' | head -100Length of output: 7343
🏁 Script executed:
#!/bin/bash
# Search for template file in channel-list-container and activeChannel bindings
fd -e html -path '*channel-list-container*' -x cat {}Length of output: 168
🏁 Script executed:
#!/bin/bash
# Look for store selectors and how activeChannel is used
fd -path '*store*' -name '*.selectors.ts' -o -name '*selector*' | head -20
rg -nP 'selectActive' --type=ts -B3 -A3Length of output: 21808
🏁 Script executed:
#!/bin/bash
# Find channel-list-container template
find . -path '*channel-list-container*' -name '*.html' 2>/dev/null -exec cat {} \;Length of output: 7334
🏁 Script executed:
#!/bin/bash
# Search for activeChannel comparisons in templates and component logic
rg -n 'activeChannel' --type-list | head -20
rg -n 'activeChannel.*\[' --type-add 'hbs:*.html' -A2 -B2
rg -n 'activeChannel' -g '*.html' -A2 -B2Length of output: 9561
🏁 Script executed:
#!/bin/bash
# Check the reducer state type and active property to understand what's stored
rg -n 'state\.active\s*=' -C3 | head -80Length of output: 0
Fix adjacent channel navigation to use URL instead of ID for consistency.
The migration to store-backed activeChannel signal is sound, and the template correctly uses URL-based comparisons (activeChannel()?.url === channel?.url). However, there's a critical inconsistency: effects.ts line 375 uses channel.id === activeChannel?.id when finding the adjacent channel, while the UI bindings and favorites system consistently use channel.url. This will cause selection mismatches when navigating to next/previous channels.
Update effects.ts line 375 to use URL matching:
(channel) => channel.url === activeChannel?.url
This ensures the adjacent channel logic aligns with the template bindings and favorites storage, which both use URLs as the selection key.
🤖 Prompt for AI Agents
In libs/ui/components/src/lib/effects.ts around line 375, the adjacent-channel
lookup currently compares channel IDs which is inconsistent with the rest of the
UI that uses channel URLs; update the predicate to compare the channel.url
against the active channel's url (use optional chaining/undefined checks as
appropriate) so the adjacent-channel selection uses URL matching (channel.url
=== activeChannel?.url) and handle the case where activeChannel may be
undefined.
4gray
force-pushed
the
feat-remote-control-ui-server-integration
branch
from
efee5ea to
a3f056d
Compare
4gray
force-pushed
the
feat-remote-control-ui-server-integration
branch
from
a3f056d to
b88078c
Compare
4gray
force-pushed
the
feat-remote-control-ui-server-integration
branch
from
b88078c to
0823af2
Compare
Introduce a shared InjectionToken for OverlayRef used by the multi-EPG overlay component and export it from the components module. - Add overlay-ref.token.ts defining COMPONENT_OVERLAY_REF to provide OverlayRef for dynamically created overlay components. - Replace a locally created InjectionToken with an injected COMPONENT_OVERLAY_REF in multi-epg-container.component to avoid token duplication and ensure consistent DI across modules. - Export the token from libs/ui/components/src/index.ts so other consumers can import and use the same token. This reduces duplicated token definitions, fixes DI inconsistencies, and makes the overlay ref reusable by external components.
Add a new library libs/ui/remote-control including component, template, Jest setup, and project configuration to provide a remote-control UI component. - Add RemoteControlComponent implemented with inject-based service usage, loading and error state, and async channelUp/channelDown handlers. This centralizes remote control logic and handles errors for the UI. - Add HTML template for the remote-control touchpad, buttons, error display, and basic branding markup. - Add Jest config (jest.config.ts) and tsconfig.spec.json configured for unit tests with jest-preset-angular to enable testing of Angular components. - Add Nx project configuration (project.json) with test and lint targets so the library is discoverable and runnable by the workspace. These changes scaffold the remote-control feature as a reusable UI library and enable testing and linting within the monorepo.
Add a new remote-control-web application using Angular standalone bootstrap. Add main.ts to bootstrap Application with appConfig and log bootstrap errors. Introduce global styles (styles.scss) to reset page margins, ensure full-height layout, set dark background, and prevent horizontal scroll. Add Nx project configuration (project.json) for build, serve, test, lint, extract-i18n and serve-static targets with production and development configurations, asset and style wiring, and Jest test integration. Add tsconfig.spec.json to configure testing compilation (Jest, Node), include test files and test setup. Start app configuration provider file (app.config.ts) to prepare application providers (partial file added). These changes scaffold the new remote-control-web application and enable building, serving and testing in the monorepo.
Update default remote control port from 3000 to 8765 in settings store and component initialization to avoid conflicts with common development ports and align with the desktop/tauri environment. Unhide and scope the remote control UI to desktop builds only: - Restore the remote control block in settings template and guard it with @if (isDesktop) so the checkbox and description render for desktop users (previously commented out as not ported). - Keep the remote control settings section conditionally visible when the remoteControl form value is true. These changes ensure desktop users can configure and enable the remote control feature with a non-conflicting default port.
Add a new HttpServer class that serves the built remote-control-web static assets and provides a simple REST/API routing layer for remote control endpoints. Key changes: - Implement server lifecycle: start, stop, updateSettings with port and enabled handling. - Resolve distribution path depending on electron packaging (dev vs production). - Serve static files with basic security (normalize path to prevent traversal) and SPA fallback to index.html for client-side routing. - Provide registration API for remote control handlers mapped to /api/remote-control/* endpoints. - Log server status and listening URL on start, and on stop. These changes enable the Electron backend to host the remote control web app and route API calls without external web servers, simplifying development and packaging.
- Replace local selected state with store signal (selectActive) in the channel list container so templates bind to activeChannel()?.url instead of comparing selected?.id. This unifies selection handling with the global store and prevents stale local state. - Update selectChannel to dispatch setActiveChannel instead of mutating local selected property; keep EPG lookup behavior intact. - Import selectActive alongside other selectors from m3u-state and remove unused local InjectionToken in video player. - Add remote control listener in VideoPlayer (Electron) to handle channel up/down events and wire a new handleRemoteChannelChange method skeleton. Use combineLatest/take to safely read required observables for channel switching. - Minor rxjs import and formatting adjustments. These changes centralize active channel state in the store, simplify template logic, and add support for remote channel changes on desktop.
Update English translations for the remote-control UI and change the
documented default remote-control port.
- Change default remote control port description from 3000 to 8765 to
reflect actual configuration.
- Improve remote control UI copy:
- Add SUBTITLE "Control your IPTV player remotely".
- Capitalize UP_CHANNEL and DOWN_CHANNEL labels to "Channel Up" and
"Channel Down" for consistent UI presentation.
These changes clarify the feature and align text with current behavior.
4gray
force-pushed
the
feat-remote-control-ui-server-integration
branch
from
0291fa8 to
63f8006
Compare
Summary
What changed
Notes
This is part 1 of 2 in a stack made with GitButler:
Summary by CodeRabbit
Release Notes