Skip to content

Add missing function call in module.js#2385

Merged
MichMich merged 4 commits intoMagicMirrorOrg:developfrom
rejas:missing_function_call
Dec 29, 2020
Merged

Add missing function call in module.js#2385
MichMich merged 4 commits intoMagicMirrorOrg:developfrom
rejas:missing_function_call

Conversation

@rejas
Copy link
Collaborator

@rejas rejas commented Dec 29, 2020

I noticed the show method is missing a () in the show method when calling the callback. The hide method has it for example.

Also cleaned up the jsdoc and adjusted some log levels

@rejas rejas changed the base branch from master to develop December 29, 2020 17:53
@sdetweil
Copy link
Collaborator

I don't think the log level needs to be warning on the open whitelist.
its not bad

@rejas
Copy link
Collaborator Author

rejas commented Dec 29, 2020

its not bad

It isnt bad per se but the log is in the warning color already. And why not show the user if he opens his ip list a lot? A more experienced user will know how to change his log level IF it really annoys him ;-)

@MichMich MichMich merged commit a857412 into MagicMirrorOrg:develop Dec 29, 2020
@fewieden
Copy link
Contributor

fewieden commented Jan 1, 2021

@rejas nice. Stumbled over the issue today as well.

@rejas
Copy link
Collaborator Author

rejas commented Jan 1, 2021

@fewieden so someone is using that call actually ;-) ?

@rejas rejas deleted the missing_function_call branch January 1, 2021 13:44
@fewieden
Copy link
Contributor

fewieden commented Jan 1, 2021

@rejas yes, in order to only blur other modules when I actually can show the modal here https://github.com/fewieden/MMM-Modal/pull/7/files#diff-62b05431c54d3d6bceb46bb4e84772f44070a474852a5272e9e468bc0692299bR329-R333

Ideally, we also trigger the callback with an error here if the lock strings prevent from showing and not only on success.
I would like to send another module a notification in that case.

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.

4 participants