Skip to content

Comments

fix(display): correct module trigger logic and ensure header visibility#45

Open
dfl-aeb wants to merge 1 commit intowonderslug:mainfrom
dfl-aeb:fix/displayed-header-even-if-empty
Open

fix(display): correct module trigger logic and ensure header visibility#45
dfl-aeb wants to merge 1 commit intowonderslug:mainfrom
dfl-aeb:fix/displayed-header-even-if-empty

Conversation

@dfl-aeb
Copy link

@dfl-aeb dfl-aeb commented May 28, 2025

Fix: Module Trigger Logic and Header Visibility

Problem:

  • Module headers were incorrectly displayed even when the module itself was supposed to be hidden by the moduleTriggerTemplate. This led to overlapping headers in the user interface.
image
  • There was a typo in the configuration option (useModuleTigger instead of useModuleTrigger).

Solution:

This PR addresses the issues mentioned above with the following changes:

  1. Corrected Header Visibility: The getHeader function in MMM-HomeAssistantDisplay.js has been adjusted to return null (and thus render no header) when the module is hidden via useModuleTrigger (or useModuleTigger).
  2. Typo Correction: The typo useModuleTigger has been corrected to useModuleTrigger throughout the module file (MMM-HomeAssistantDisplay.js).
  3. Backward Compatibility: To avoid breaking existing configurations, backward compatibility for the misspelled useModuleTigger option has been implemented. The module now checks for both spellings (useModuleTrigger || useModuleTigger).

Impact:

  • Hidden modules will now correctly no longer display their headers, resolving the issue of overlapping headers.
  • The configuration is now more consistent due to the typo correction.
  • Existing configurations using the useModuleTigger typo will continue to work without requiring modification.

 This commit addresses an issue where module headers were displayed even when the module itself was hidden by the `moduleTriggerTemplate`. It also corrects a typo in the `useModuleTrigger` configuration option.  Additionally, backward compatibility for the misspelled `useModuleTigger` option has been introduced to prevent breaking changes for existing configurations.
@wonderslug wonderslug requested a review from Copilot August 2, 2025 20:50
@wonderslug wonderslug added the bug Something isn't working label Aug 2, 2025
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes display issues in the MMM-HomeAssistantDisplay module by correcting module trigger logic and ensuring proper header visibility control. The changes address overlapping headers when modules should be hidden and fix a configuration option typo.

  • Corrected typo from useModuleTigger to useModuleTrigger while maintaining backward compatibility
  • Fixed header visibility logic to hide headers when modules are triggered to be hidden
  • Updated trigger condition checks throughout the module to use corrected logic

debuglogging: false,
useModuleTigger: false,
useModuleTrigger: false, // Corrected typo: useModuleTigger -> useModuleTrigger
useModuleTigger: false, // For backwards compatibility
Copy link

Copilot AI Aug 2, 2025

Choose a reason for hiding this comment

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

The comment maintains the intentional misspelling 'Tigger' for backward compatibility, but it should clarify that this is a deprecated option. Consider updating the comment to: '// For backwards compatibility - deprecated, use useModuleTrigger instead'

Suggested change
useModuleTigger: false, // For backwards compatibility
useModuleTigger: false, // For backwards compatibility - deprecated, use useModuleTrigger instead

Copilot uses AI. Check for mistakes.
// If the module is configured to use a trigger to control its visibility,
// and the trigger currently indicates the module should be hidden,
// then do not return a title, so no header is rendered.
if ((this.config.useModuleTrigger || this.config.useModuleTigger) && !this.displayModule) {
Copy link

Copilot AI Aug 2, 2025

Choose a reason for hiding this comment

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

The condition checks !this.displayModule but displayModule is not defined or initialized in the visible code. This could result in the header always being hidden if displayModule is undefined. Ensure displayModule is properly initialized and updated based on the module trigger logic.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants