Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Weird broadcasted form of multi symbols of calendar module. #3267

Closed
eouia opened this issue Nov 12, 2023 · 4 comments · Fixed by #3673
Closed

Weird broadcasted form of multi symbols of calendar module. #3267

eouia opened this issue Nov 12, 2023 · 4 comments · Fixed by #3673

Comments

@eouia
Copy link
Contributor

eouia commented Nov 12, 2023

I remember it was not so, but multiple symbols are broadcasted weirdly at some point.

When I set some symbols in a calendar like this;

symbol: ["file", "calendar"],

The expected broadcasted data(And the historical format what I remember) would be like this; (with defaultSymbolClassName)

symbol: ["fa fa-fw fa-file", "fa fa-fw fa-calendar"],
// Array(2)

Bur really broadcasted would be;

symbol: ["fa fa-fw fa-file, fa fa-fw fa-calendar"],
// Array(1)

Is this intentionally designed so? Or just an unexpected accident?

@rejas
Copy link
Collaborator

rejas commented Nov 18, 2023

Thanks for the report (and sorry for the late replay). Can you pinpoint when this behaviour changed?

@sdetweil
Copy link
Collaborator

I am fixing things in calendar now.. @eouia can u answer @rejas

@sdetweil
Copy link
Collaborator

I see the bug, checking on how to fix

@sdetweil
Copy link
Collaborator

so, here is my proposed change

	getCalendarPropertyAsArray (url, property, defaultValue) {
		let p = this.getCalendarProperty(url, property, defaultValue);
		if (property === "symbol" || property === "recurringSymbol" || property === "fullDaySymbol") {
			const className = this.getCalendarProperty(url, "symbolClassName", this.config.defaultSymbolClassName);
			if(p instanceof Array)       //  add
				p.push(className) // add
			else   // add
				p = className + p;  // old code assumed property was string
		}

		if (!(p instanceof Array)) p = [p];
		return p;
	},

@sdetweil sdetweil self-assigned this Oct 21, 2024
sdetweil added a commit to sdetweil/MagicMirror that referenced this issue Dec 8, 2024
rejas pushed a commit that referenced this issue Dec 8, 2024
this change was dropped from #3267 by mistake
sdetweil added a commit that referenced this issue Jan 1, 2025
## [2.30.0] - 2025-01-01

Thanks to: @xsorifc28, @HeikoGr, @bugsounet, @khassel,
@KristjanESPERANTO, @rejas, @sdetweil.

> ⚠️ This release needs nodejs version `v20` or `v22 or higher`, minimum
version is `v20.18.1`

### Added

- [core] Add wayland and windows start options to `package.json` (#3594)
- [docs] Add step for npm publishing in release process (#3595)
- [core] Add GitHub workflow to run spellcheck a few days before each
release (#3623)
- [core] Add test flag to `index.html` to pass to module js for test
mode detection (needed by #3630)
- [core] Add export on animation names (#3644)
- [compliments] Add support for refreshing remote compliments file, and
test cases (#3630)
- [linter] Re-add `eslint-plugin-import`now that it supports ESLint v9
(#3586)
- [linter] Re-activate `eslint-plugin-package-json` to lint
`package.json` (#3643)
- [linter] Add linting for markdown files (#3646)
- [linter] Add some handy ESLint rules.
- [calendar] Add ability to display end date for full date events, where
end is not same day (showEnd=true) (#3650)
- [core] Add text to the config.js.sample file about the locale variable
(#3654, #3655)
- [core] Add fetch timeout for all node_helpers (thru undici, forces
node 20.18.1 minimum) to help on slower systems. (#3660) (3661)

### Changed

- [core] Run code style checks in workflow only once (#3648)
- [core] Fix animations export #3644 only on server side (#3649)
- [core] Use project URL in fallback config (#3656)
- [core] Fix Access Denied crash writing js/positions.js (on synology
nas) #3651. new message, MM starts, but no modules showing (#3652)
- [linter] Switch to 'npx' for lint-staged in pre-commit hook (#3658)

### Removed

- [tests] Remove `node-pty` and `drivelist` from rebuilded test (#3575)
- [deps] Remove `@eslint/js` dependency. Already installed with `eslint`
in deep (#3636)

### Updated

- [repo] Reactivate `stale.yaml` as GitHub action to mark issues as
stale after 60 days and close them 7 days later (if no activity) (#3577,
#3580, #3581)
- [core] Update electron dependency to v32 (test electron rebuild) and
all other dependencies too (#3657)
- [tests] All test configs have been updated to allow full external
access, allowing for easier debugging (especially when running as a
container)
- [core] Run and test with node 23 (#3588)
- [workflow] delete exception `allow-ghsas: GHSA-8hc4-vh64-cxmj` in
`dep-review.yaml` (#3659)

### Fixed

- [updatenotification] Fix pm2 using detection when pm2 script is inside
or outside MagicMirror root folder (#3576) (#3605) (#3626) (#3628)
- [core] Fix loading node_helper of modules: avoid black screen, display
errors and continue loading with next module (#3578)
- [weather] Change default value for weatherEndpoint of provider
openweathermap to "/onecall" (#3574)
- [tests] Fix electron tests with mock dates, the mock on server side
was missing (#3597)
- [tests] Fix testcases with hard coded Date.now (#3597)
- [core] Fix missing `basePath` where `location.host` is used (#3613)
- [compliments] croner library changed filenames used in latest version
(#3624)
- [linter] Fix ESLint ignore pattern which caused that default modules
not to be linted (#3632)
- [core] Fix module path in case of sub/sub folder is used and use
path.resolve for resolve `moduleFolder` and `defaultModuleFolder` in
app.js (#3653)
- [calendar] Update to resolve issues #3098 #3144 #3351 #3422 #3443
#3467 #3537 related to timezone changes
- [calendar] Fix #3267 (styles array), also fixes event with both exdate
AND recurrence(and testcase)
- [calendar] Fix showEndsOnlyWithDuration not working, #3598, applies
ONLY to full day events
- [calendar] Fix showEnd for Full Day events (#3602)
- [tests] Suppress "module is not defined" in e2e tests (#3647)
- [calendar] Fix #3267 (styles array, really this time!)
- [core] Fix #3662 js/positions.js created incorrectly

---------

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: Karsten Hassel <[email protected]>
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: 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]>
Co-authored-by: HeikoGr <[email protected]>
Co-authored-by: Pedro Lamas <[email protected]>
Co-authored-by: veeck <[email protected]>
khassel pushed a commit that referenced this issue Jan 5, 2025
fixes #3267 AGAIN, correctly, add testcase
add testcase for #3679 , broadcast clipping incorrectly

I added a test module (tests/testNotification) to catch the notification
and check the count. (one way to configure)
i put this module in the tests folder, and added /tests to the server
paths.
(can't have a module in a nested folder, like tests/modules/xxx)

but I have a problem. i can run the test config (MM_CONFIG_FILE), and
the two modules work correctly,
but in the spec runner, the calendar module times out on the broadcast
test.. there is only local ics file access, no outside hosts

I forced my system date to 1/1/24 (same as runner) and again the manual
testcase works fine

I added two test config.js,(configs/calendar) one works great (symbols)
, one fails (broadcast test)
I added additional delay in the calendarspec runner to try to debug the
module, but it still not long enough.. no messages of trouble when I get
into the browser.. BUT, this may be because of the log being turned
off... (just thought of this)

I created a special ICS (in mocks) that has 12 events, 1 for each
month.. (so I can check clipping and broadcast) the US holidays one is
sensitive to the current date, and I couldn't get it to work on
1/1/2024..

also, in general, is there a mechanism to run test:just_one_runner?
waiting thru the electron test to get to one testcase.. ugh..
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants