add support for custom regions, by detecting what is used in index.html by sdetweil · Pull Request #3518 · MagicMirrorOrg/MagicMirror

sdetweil · 2024-08-04T15:46:37Z

read index.html to discover the regions used, make them the list checked by app.js and check:config test

fixes #3504 supercedes #3506

no config.js param required

sdetweil · 2024-08-04T15:49:37Z

how do I create a testcase where I need to replace index.html??
I need 3 testcases

bad position name (standard index.html)
custom position, config matching, need new index.html
custom position, config wth new position mispelled, for error of new, need new index.html (same as test 2)

I removed the hard coded position names from the code... all is detected

khassel · 2024-08-04T17:07:01Z

In e2e weather tests is a similar construction where a string #####WEATHERDATA##### is replaced in config.js, you can search for weather_mocker in the test folder.

I think you have to restore the original index.html after your tests (e.g. bei calling git checkout index.html).

sdetweil · 2024-08-04T17:09:37Z

that replaces the string in the config.js
I understand I would have to restore it.. just haven't seen a file replace as we can specify where the config is thru env vars...

khassel · 2024-08-04T17:16:28Z

yes, we have no such file replace, maybe simple solution is

load file content
replace (e.g. valid position with invalid)
save changed file content
test
revert changes

khassel · 2024-08-04T20:13:07Z

supercedes #3506

what about the changes done in the above PR? Do we still need allowCustomModulePositions or should that commit be reverted?

sdetweil · 2024-08-04T20:16:47Z

i removed it.. (removed the code, not the commit)

still testing.. testcase fails.. not sure why..

sdetweil · 2024-08-04T20:48:27Z

ok.. one more problem on this

so I filtered the regions to positions and we went thru config , and checked.. all good..

start server, open web window, on index.html..
main.js loads all the module js files.. oops.. it has a hard coded list of positions.. doah..

so, i have to pass the generated list
include another js file for it to load?
which has the array definition of the found positions?

this works..

new file
js/positions.js
const modulePositions = [......]

comment out the same in main.js

any objections to this approach?
rewrite the js/positions.js file on each startup (ugh)

sdetweil · 2024-08-04T20:59:36Z

is there a testcase that captures error

-08-04 15:57:54.546] [WARN]  Invalid module position found for this configuration:
{
  "module": "helloworld",
  "disabled": false,
  "position": "row3_left1",

as the position is bad, MM doesn't load the module, there is no web content.. so we wait til timeout..

khassel · 2024-08-04T21:06:25Z

is there a testcase that captures error

I think no looking at #3445

khassel · 2024-08-04T21:11:28Z

any objections to this approach?

not sure, I did similar ugly things in https://github.com/MagicMirrorOrg/MagicMirror/pull/3231/files replacing a placeholder with content of a config property

sdetweil · 2024-08-04T21:16:09Z

ok.. I will be gone for a week (mon-mon) so won't work on it til I get back.. just have to figure out verifying the bad position requested.. have good now... I will push all this so not lost

… customregions

sdetweil · 2024-08-18T17:45:27Z

this is ready for review

sdetweil · 2024-08-20T16:05:01Z

what is this failure? I didn't change anything in the on/off rebuild test

js/app.js

tests/configs/customregions.js

js/utils.js

khassel · 2024-08-26T18:47:18Z

and I think allowCustomModulePositions: false, should be removed from js/defaults.js

sdetweil · 2024-08-26T18:52:50Z

ok missed that.

sdetweil · 2024-08-27T15:18:07Z

ok, updated to remove that

Should have been part of MagicMirrorOrg#3518

This file is generated when MM is started. As I understand it, it should not be included in the repository. Should probably have been part of #3518.

bugsounet · 2024-09-28T09:02:03Z

@sdetweil : I just discover a bug in getModulePositions()

This function is called 2 times in app.js

1st time in start of MM with require(${global.root_path}/js/check_config.js); (L120)
2nd time in this.start with Utils.getModulePositions() (L272)

Result:
modulePositions have duplicate positions and write the positions.js file 2 times

just check position.js file
const modulePositions=["fullscreen_below","top_bar","top_left","top_center","top_right","upper_third","middle_center","lower_third","bottom_bar","bottom_left","bottom_center","bottom_right","fullscreen_above","fullscreen_below","top_bar","top_left","top_center","top_right","upper_third","middle_center","lower_third","bottom_bar","bottom_left","bottom_center","bottom_right","fullscreen_above"]

sdetweil · 2024-09-28T11:30:57Z

thats because check:config was just added to the startup.

bugsounet · 2024-09-28T11:36:29Z

I know sam, i'm just reporting what I've noticed.

Maybe removing L272 and using only check config will do the job

sdetweil · 2024-09-28T12:10:23Z

i didn't add check config.

bugsounet · 2024-09-28T12:23:03Z

You have right sam,

it's @KristjanESPERANTO #3450

sdetweil · 2024-09-28T12:42:20Z

or we could add a check in getModulePositions if the modulePositions is not populated

sdetweil · 2024-09-28T13:41:48Z

i added a pr for the latter, check if work already done, in case we remove check:config from startup

bugsounet · 2024-09-28T13:55:33Z

it seems to do the job ;)

@bugsounet

## [2.29.0] - 2024-10-01 Thanks to: @bugsounet, @dkallen78, @jargordon, @khassel, @KristjanESPERANTO, @MarcLandis, @rejas, @ryan-d-williams, @sdetweil, @skpanagiotis. > ⚠️ This release needs nodejs version `v20` or `v22`, minimum version is `v20.9.0` ### Added - [compliments] Added support for cron type date/time format entries mm hh DD MM dow (minutes/hours/days/months and day of week) see https://crontab.cronhub.io for construction (#3481) - [core] Check config at every start of MagicMirror² (#3450) - [core] Add spelling check (cspell): `npm run test:spelling` and handle spelling issues (#3544) - [core] removed `config.paths.vendor` (could not work because `vendor` is hardcoded in `index.html`), renamed `config.paths.modules` to `config.foreignModulesDir`, added variable `MM_CUSTOMCSS_FILE` which - if set - overrides `config.customCss`, added variable `MM_MODULES_DIR` which - if set - overrides `config.foreignModulesDir`, added test for `MM_MODULES_DIR` (#3530) - [core] elements are now removed from `index.html` when loading script or stylesheet files fails - [core] Added `MODULE_DOM_UPDATED` notification each time the DOM is re-rendered via `updateDom` (#3534) - [tests] added minimal needed node version to tests (currently v20.9.0) to avoid releases with wrong node version info - [tests] Added `node-libgpiod` library to electron-rebuild tests (#3563) ### Removed - [core] removed installer only files (#3492) - [core] removed raspberry object from systeminformation (#3505) - [linter] removed `eslint-plugin-import`, because it doesn't support ESLint v9. We will reenter it later when it does. - [tests] removed `onoff` library from electron-rebuild tests (#3563) ### Updated - [weather] Updated `apiVersion` default from 2.5 to 3.0 (#3424) - [core] Updated dependencies including stylistic-eslint - [core] nail down `node-ical` version to `0.18.0` with exception `allow-ghsas: GHSA-8hc4-vh64-cxmj` in `dep-review.yaml` (which should removed after next `node-ical` update) - [core] Updated SocketIO catch all to new API - [core] Allow custom modules positions by scanning index.html for the defined regions, instead of hard coded (PR #3518 fixes issue #3504) - [core] Detail optimizations in `config_check.js` - [core] Updated minimal needed node version in `package.json` (currently v20.9.0) (#3559) and except for v21 (no security updates) (#3561) - [linter] Switch to ESLint v9 and flat config and replace `eslint-plugin-unicorn` by `@eslint/js` - [core] fix discovering module positions twice after #3450 ### Fixed - Fixed `checks` badge in README.md - [weather] Fixed issue with the UK Met Office provider following a change in their API paths and header info. - [core] add check for node_helper loading for multiple instances of same module (#3502) - [weather] Fixed issue for respecting unit config on broadcasted notifications - [tests] Fixes calendar test by moving it from e2e to electron with fixed date (#3532) - [calendar] fixed sliceMultiDayEvents getting wrong count and displaying incorrect entries, Europe/Berlin (#3542) - [tests] ignore `js/positions.js` when linting (this file is created at runtime) - [calendar] fixed sliceMultiDayEvents showing previous day without config enabled --------- Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: Michael Teeuw <[email protected]> Co-authored-by: Kristjan ESPERANTO <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Ross Younger <[email protected]> Co-authored-by: Veeck <[email protected]> Co-authored-by: Bugsounet - Cédric <[email protected]> Co-authored-by: jkriegshauser <[email protected]> Co-authored-by: illimarkangur <[email protected]> Co-authored-by: sam detweiler <[email protected]> Co-authored-by: vppencilsharpener <[email protected]> Co-authored-by: veeck <[email protected]> Co-authored-by: Paranoid93 <[email protected]> Co-authored-by: Brian O'Connor <[email protected]> Co-authored-by: WallysWellies <[email protected]> Co-authored-by: Jason Stieber <[email protected]> Co-authored-by: jargordon <[email protected]> Co-authored-by: Daniel <[email protected]> Co-authored-by: Ryan Williams <[email protected]> Co-authored-by: Panagiotis Skias <[email protected]> Co-authored-by: Marc Landis <[email protected]>

add support for custom regions, by detecting what is used in index.html

2e9b0ae

Merge branch 'develop' into customregions

76ba806

sdetweil added 5 commits August 4, 2024 16:30

add fix for main.js having hard coded positions, start on testcases

a7d05ca

Merge remote-tracking branch 'refs/remotes/origin/customregions' into…

c9cfb9b

… customregions

fix changelog

e579df5

add testcases for custom module regions

7fe1f50

fix race condition in testcase

4afdd25

sdetweil added 2 commits August 20, 2024 10:52

Merge branch 'develop' into customregions

d94d84b

fix CHANGELOG for interim PR merges

8ce819e

sdetweil mentioned this pull request Aug 20, 2024

allow custom module positions #3231

Closed

rejas reviewed Aug 21, 2024

View reviewed changes

js/app.js Show resolved Hide resolved

tests/configs/customregions.js Show resolved Hide resolved

khassel reviewed Aug 26, 2024

View reviewed changes

js/utils.js Show resolved Hide resolved

remove property no longer used

0e44d57

khassel merged commit 2b97e0d into MagicMirrorOrg:develop Aug 27, 2024

KristjanESPERANTO added a commit to KristjanESPERANTO/MagicMirror that referenced this pull request Sep 11, 2024

Ignore positions.js

03bc9ed

Should have been part of MagicMirrorOrg#3518

KristjanESPERANTO mentioned this pull request Sep 11, 2024

Ignore positions.js #3531

Merged

KristjanESPERANTO added a commit to KristjanESPERANTO/MagicMirror that referenced this pull request Sep 11, 2024

Ignore positions.js

549e09f

Should have been part of MagicMirrorOrg#3518

khassel pushed a commit that referenced this pull request Sep 11, 2024

Ignore positions.js (#3531)

bca5d9c

This file is generated when MM is started. As I understand it, it should not be included in the repository. Should probably have been part of #3518.

khassel mentioned this pull request Sep 30, 2024

Release 2.29.0 #3568

Merged

sdetweil deleted the customregions branch October 5, 2024 16:33

timmymac-tech mentioned this pull request Oct 6, 2024

Release 2.29.0 (#3568) timmymac-tech/MagicMirror#2

Merged

Protonosgit mentioned this pull request Oct 6, 2024

Release 2.29.0 (#3568) Protonosgit/MagicMedia#1

Merged

Uh oh!

Conversation

sdetweil commented Aug 4, 2024

Uh oh!

sdetweil commented Aug 4, 2024

Uh oh!

khassel commented Aug 4, 2024

Uh oh!

sdetweil commented Aug 4, 2024

Uh oh!

khassel commented Aug 4, 2024

Uh oh!

khassel commented Aug 4, 2024

Uh oh!

sdetweil commented Aug 4, 2024

Uh oh!

sdetweil commented Aug 4, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sdetweil commented Aug 4, 2024

Uh oh!

khassel commented Aug 4, 2024

Uh oh!

khassel commented Aug 4, 2024

Uh oh!

sdetweil commented Aug 4, 2024

Uh oh!

sdetweil commented Aug 18, 2024

Uh oh!

sdetweil commented Aug 20, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

khassel commented Aug 26, 2024

Uh oh!

sdetweil commented Aug 26, 2024

Uh oh!

sdetweil commented Aug 27, 2024

Uh oh!

bugsounet commented Sep 28, 2024

Uh oh!

sdetweil commented Sep 28, 2024

Uh oh!

bugsounet commented Sep 28, 2024

Uh oh!

sdetweil commented Sep 28, 2024

Uh oh!

bugsounet commented Sep 28, 2024

Uh oh!

sdetweil commented Sep 28, 2024

Uh oh!

sdetweil commented Sep 28, 2024

Uh oh!

bugsounet commented Sep 28, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

sdetweil commented Aug 4, 2024 •

edited

Loading