Skip to content

Refactor: Prevent nchan updates on browsers not in focus#2081

Merged
limetech merged 10 commits into
unraid:masterfrom
Squidly271:nchan-fix
Mar 24, 2025
Merged

Refactor: Prevent nchan updates on browsers not in focus#2081
limetech merged 10 commits into
unraid:masterfrom
Squidly271:nchan-fix

Conversation

@Squidly271

@Squidly271 Squidly271 commented Mar 22, 2025

Copy link
Copy Markdown
Contributor

Summary by CodeRabbit

  • New Features
    • Enhanced dashboard monitoring to provide more robust performance insights.
    • Introduced an option in display settings to enable or disable real-time updates when the browser is inactive.
    • Improved live update management by automatically clearing outdated performance data when navigating away from the page.
    • Added monitoring capabilities for Docker loading processes and VM usage statistics.
    • Consistent monitoring activation for device and state management processes.

@coderabbitai

coderabbitai Bot commented Mar 22, 2025

Copy link
Copy Markdown
Contributor

Walkthrough

This update enhances several components in the Dynamix plugin. The dashboard-related objects now invoke a monitoring method after starting, and a window blur event resets key data arrays. A server post call has been disabled. Additionally, copyright years were updated, and a new form dropdown was introduced to control real-time updates on inactive browsers. A corresponding configuration entry was added to disable live updates by default. Finally, the DefaultPageLayout now handles Nchan subscribers with a new monitor method and improved focus/blur events for managing live updates.

Changes

File Path Change Summary
emhttp/.../DashStats.page Updated dashboard, vmdashusage, and apcups calls to use .start().monitor(). Added a window blur handler to reset cpu, rxd, and txd arrays, and commented out the server post request.
emhttp/.../DisplaySettings.page Updated copyright years to 2025; added a new select dropdown ("Allow realtime updates on inactive browsers") using the liveUpdate option.
emhttp/.../default.cfg Introduced a new configuration entry: liveUpdate="no" in the [display] section.
emhttp/.../include/DefaultPageLayout.php Added NchanSubscriber.prototype.monitor and a global subscribers array; modified live update control with window focus/blur events and replaced setTimeout with setInterval.
emhttp/.../DockerContainers.page Updated dockerload.start() to use .monitor(). Removed the window.onunload function that previously stopped the Docker loading process.
emhttp/.../VMUsageStats.page Updated vmusage.start() to use .monitor(). Removed the window.onunload function that previously stopped the VM usage monitoring.
emhttp/.../ArrayOperation.page Modified toggle_state, mymonitor, devices, and fsState functions to include .monitor() after starting respective processes.

Possibly related PRs

  • Wireless support - fine tuning #2002: The changes in the main PR, which enhance monitoring capabilities for various objects by appending .monitor() to their start() method calls, are related to the modifications in the retrieved PR that also involve adding .monitor() to the dockerload.start() method, indicating a direct connection in the code level changes.
  • Wireless: add wifi info to dashboard and info button #2038: The changes in the main PR, which enhance monitoring capabilities for various objects by appending .monitor() to their start() method calls, are related to the modifications in the retrieved PR that also involve adding a .monitor() method to the dockerload object, indicating a similar enhancement in monitoring functionality.
  • Wireless and VM improvements #2007: The changes in the main PR, which enhance monitoring capabilities for various objects by appending .monitor() to their start() method calls, are related to the modifications in the retrieved PR that also involve adding a .monitor() method to the vmusage object in the VMUsageStats.page file. Both PRs focus on enhancing monitoring functionality in their respective components.

Poem

I'm a rabbit, hopping through code trails bright,
Monitoring now adds a new sight.
When the window fades, data clears away,
And live updates pause till you come to play.
Cheerful hops for changes done right! 🐇✨


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)

📥 Commits

Reviewing files that changed from the base of the PR and between 38efaf4 and 0c33f55.

📒 Files selected for processing (6)
  • emhttp/plugins/dynamix.docker.manager/DockerContainers.page (1 hunks)
  • emhttp/plugins/dynamix.vm.manager/VMUsageStats.page (2 hunks)
  • emhttp/plugins/dynamix/ArrayOperation.page (4 hunks)
  • emhttp/plugins/dynamix/DashStats.page (2 hunks)
  • emhttp/plugins/dynamix/DisplaySettings.page (2 hunks)
  • emhttp/plugins/dynamix/include/DefaultPageLayout.php (3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • emhttp/plugins/dynamix/DisplaySettings.page
🔇 Additional comments (18)
emhttp/plugins/dynamix.vm.manager/VMUsageStats.page (2)

7-8: Updated copyright years to 2025.

The copyright years have been updated from 2024 to 2025 for both Lime Technology and Simon Fairweather.


35-35: Added monitoring capability to the vmusage subscriber.

The code now calls .monitor() after .start(), which adds the subscriber to a global tracking array for proper focus/blur management. This is consistent with the PR objective to prevent Nchan updates on browsers that aren't in focus.

emhttp/plugins/dynamix.docker.manager/DockerContainers.page (1)

195-195: Added monitoring capability to the dockerload subscriber.

The code now calls .monitor() after .start(), which adds the subscriber to a global tracking array for proper focus/blur management. This change aligns with the PR objective to prevent unnecessary updates when the browser tab isn't in focus.

emhttp/plugins/dynamix/ArrayOperation.page (4)

117-117: Added monitoring capability to the devices subscriber in toggle_state function.

The toggle_state function now calls .monitor() after .start(), ensuring that the devices subscriber is tracked for focus/blur management. This change is part of the broader implementation to prevent unnecessary Nchan updates on inactive browser tabs.


367-367: Added monitoring capability to the mymonitor subscriber.

The code now calls .monitor() after .start(), adding the mymonitor subscriber to the global tracking array. This follows the pattern applied to other subscribers in the PR.


423-423: Added monitoring capability to the devices subscriber.

The code now calls .monitor() after .start(), adding the devices subscriber to the global tracking array for focus/blur management. This is consistent with the PR's goal.


437-437: Added monitoring capability to the fsState subscriber.

The code now calls .monitor() after .start(), adding the fsState subscriber to the global tracking array for proper focus/blur management. This is in line with the PR objective to prevent Nchan updates on browsers not in focus.

emhttp/plugins/dynamix/include/DefaultPageLayout.php (7)

108-108: Added monitor method to NchanSubscriber prototype.

This new method adds the subscriber instance to the global subscribers array for tracking by the focus/blur handlers. This is a critical part of the implementation for preventing updates on inactive browser tabs.


114-115: Added global subscribers array for tracking Nchan subscribers.

This array stores references to all monitored Nchan subscribers, allowing them to be collectively paused and restarted based on browser focus events.


726-726: Modified page reload behavior to respect nchanPaused state.

The automated page reload will now only occur if nchanPaused is false, preventing unnecessary reloads when the browser tab isn't in focus. This modification also checks if $display['liveUpdate'] isn't set to "no" before setting up the interval.


1239-1241: Added nchanPaused flag for tracking focus state.

This flag tracks whether updates are currently paused due to the browser tab being out of focus, which is used by both the focus/blur handlers and the page reload logic.


1242-1256: Implemented window focus/blur event handlers conditionally.

These handlers manage starting and stopping Nchan subscribers based on browser tab focus, but only if $display['liveUpdate'] is set to "no". The implementation correctly handles the removal of warning banners and restart of subscribers when focus is regained.


1257-1265: Added window unload/beforeunload handlers for cleanup.

These handlers ensure subscribers are properly stopped when the page is unloaded or navigated away from. The comment explains that both handlers are needed for better compatibility with mobile devices.


1266-1283: Implemented nchanFocusStop function for centralized subscriber management.

This function:

  1. Stops all monitored subscribers
  2. Handles errors gracefully by removing failed subscribers from the tracking array
  3. Optionally displays a warning banner when updates are paused
  4. Prevents duplicate actions by checking the current state

The implementation is robust and handles edge cases appropriately.

emhttp/plugins/dynamix/DashStats.page (4)

1798-1798: Method chaining to enhance monitoring capabilities.

The .monitor() call has been added to the dashboard startup sequence, enabling the real-time monitoring system to respond to browser focus state changes. This improvement aligns with the PR objective of preventing unnecessary updates when browsers aren't in focus.


1800-1800: Method chaining added for VM dashboard usage monitoring.

Similar to the dashboard object, the VM dashboard usage object now has .monitor() chained to its start method, providing consistent behavior across different dashboard components.


1803-1803: Method chaining added for UPS monitoring.

The APC UPS monitoring component now includes the .monitor() method call, ensuring that all dashboard monitoring components have consistent behavior regarding browser focus states.


1819-1825: Added focus-aware data management to prevent unnecessary processing.

This new code implements the core functionality described in the PR: preventing updates when browsers aren't in focus. The window blur event handler clears chart data arrays (cpu, rxd, txd) when the browser tab loses focus, but only if the user has disabled live updates via the liveUpdate setting. This conserves resources and prevents unnecessary data accumulation when the user isn't actively viewing the dashboard.


🪧 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>, please review it.
    • 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 gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @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 using 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.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

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.

@Squidly271 Squidly271 marked this pull request as draft March 22, 2025 03:01

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
emhttp/plugins/dynamix/include/DefaultPageLayout.php (1)

1239-1270: Added window focus/blur event handlers to manage live updates

New event handlers have been implemented to stop Nchan subscribers when the window loses focus and restart them when focus is regained. This prevents unnecessary updates and potential server load for inactive browser tabs.

There's a minor issue in the error handling syntax.

On line 1259, the catch block should include the error parameter as catch(e) instead of just catch:

-        } catch {
+        } catch(e) {
           newsub.splice(newsub.indexOf(e,1));
         }
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3439b1f and 38efaf4.

📒 Files selected for processing (4)
  • emhttp/plugins/dynamix/DashStats.page (2 hunks)
  • emhttp/plugins/dynamix/DisplaySettings.page (2 hunks)
  • emhttp/plugins/dynamix/default.cfg (1 hunks)
  • emhttp/plugins/dynamix/include/DefaultPageLayout.php (3 hunks)
🔇 Additional comments (11)
emhttp/plugins/dynamix/default.cfg (1)

38-38: Added configuration option for live update control

A new configuration entry liveUpdate="no" has been added to the [display] section, which disables real-time updates for browsers not in focus by default. This aligns with the changes in the DisplaySettings page.

emhttp/plugins/dynamix/DisplaySettings.page (2)

7-8: Updated copyright years to 2025

The copyright years have been updated from 2023 to 2025 for both Lime Technology and Bergware International.


326-330: Added UI control for real-time updates on inactive browsers

A new dropdown has been added to control whether real-time updates should occur when the browser is not in focus. This corresponds to the liveUpdate option added in the default.cfg file.

emhttp/plugins/dynamix/include/DefaultPageLayout.php (3)

108-108: Added new monitor method to track Nchan subscribers

This method pushes the current Nchan subscriber instance to a global array for centralized management.


114-114: Added global array to store Nchan subscribers

A new global array subscribers has been created to keep track of all Nchan subscribers, allowing them to be controlled centrally when the window loses or gains focus.


726-726: Modified page reload to respect focus state

The page reload mechanism now checks the nchanPaused flag before reloading, preventing unnecessary reloads when the browser tab is not in focus.

emhttp/plugins/dynamix/DashStats.page (5)

1798-1798: Added monitoring capability to dashboard

The .monitor() method call adds the ability to track browser focus state to the dashboard object, which aligns with the PR objective of preventing nchan updates on inactive browsers.


1800-1800: Added monitoring capability to VM dashboard usage

Similar to the dashboard object, the VM dashboard usage object now has monitoring capabilities to detect when the browser is not in focus.


1803-1803: Added monitoring capability to APC UPS object

The UPS monitoring object is now also equipped with focus tracking capability through the .monitor() method.


1821-1821: Removed unnecessary data posting on window unload

The code previously posted chart data to the server when the window was unloaded. This has been commented out, likely to reduce unnecessary server interactions for inactive browsers.


1826-1832: Reset graph data when window loses focus

This new code block implements the core functionality of the PR by clearing graph data arrays when the browser tab loses focus, but only if the user has disabled live updates in their settings. This prevents unnecessary resource consumption and aligns perfectly with the PR objective.

@Squidly271 Squidly271 marked this pull request as ready for review March 24, 2025 18:17
@limetech limetech merged commit e5c0dac into unraid:master Mar 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants