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

Calendar module: Vertical alignment with wrapping #3053

Closed
MikeBishop opened this issue Feb 26, 2023 · 24 comments
Closed

Calendar module: Vertical alignment with wrapping #3053

MikeBishop opened this issue Feb 26, 2023 · 24 comments

Comments

@MikeBishop
Copy link
Contributor

Platform: MM2 running on Raspberry Pi 4, Bullseye

Node Version: 16.19.0

MagicMirror² Version: 2.22.0

Description: When calendar events are long enough to wrap, the event name does not have the same vertical alignment as the icon and the time string.
image

Steps to Reproduce: Add an event with a long name and/or using relative time, enable icons for calendar events

Expected Results: Consistent vertical alignment between elements, regardless of whether that's top-aligned or centered.

Actual Results: Icon is top-aligned, event name is center-aligned.

Configuration:

var config = {
	address: "0.0.0.0", // Address to listen on, can be:
	// - "localhost", "127.0.0.1", "::1" to listen on loopback interface
	// - another specific IPv4/6 to listen on a specific interface
	// - "", "0.0.0.0", "::" to listen on any interface
	// Default, when address config is left out, is "localhost"
	port: 8080,
	ipWhitelist: ["127.0.0.1", "::ffff:127.0.0.1", "::1",
		"192.168.0.0/16"], // Set [] to allow all IP addresses
	// or add a specific IPv4 of 192.168.1.5 :
	// ["127.0.0.1", "::ffff:127.0.0.1", "::1", "::ffff:192.168.1.5"],
	// or IPv4 range of 192.168.3.0 --> 192.168.3.15 use CIDR format :
	// ["127.0.0.1", "::ffff:127.0.0.1", "::1", "::ffff:192.168.3.0/28"],

	language: "en",
	timeFormat: 12,
	modules: [
		{
			module: "alert",
		},
		// {
		// 	module: "updatenotification",
		// 	position: "top_bar"
		// },
		{
			module: "clock",
			position: "top_left"
		},
		{
			module: 'calendar_monthly',
			position: 'top_right',
			config: {}
		},
		{
			module: 'uptimerobot',
			position: 'top_center',
			config: {
				api_key: "XXXXXXXXX",
				useIcons: true,
				useColors: true
			}
		},
		{
			module: 'calendar',
			position: 'top_left',
			config: {
				calendars: [
					{
						url: 'webcal://www.calendarlabs.com/ical-calendar/ics/76/US_Holidays.ics',
						symbol: 'flag',
						color: '#e42217'
					},
					{
						url: 'XXXXXXXXX',
						symbol: 'user-friends',
						color: '#fff380'
					},
					{
						url: 'XXXXXXXXX',
						symbol: 'chalkboard-user',
						color: '#047832'
					},
					{
						url: 'XXXXXXXXX',
						symbol: 'globe',
						color: '#f93'
					}
				],
				fadePoint: 0.4,
				hideTime: true,
				showTimeToday: true,
				maximumNumberOfDays: 90,
				maximumEntries: 8,
				displaySymbol: true,
				colored: true,
				coloredSymbolOnly: true,
				wrapEvents: true,
				timeFormat: "relative",
				nextDaysRelative: true,
				hideOngoing: false,
				getRelative: 0
			}
		},
		{
			module: 'MMM-WeatherOrNot',
			position: 'bottom_bar',
			config: {
				location: 'XXXXX',
				locationCode: 'XXXXXXXXX',
				languages: 'en',
				tempUnits: 'F',
				days: '7',
				icons: 'Climacons Animated',
				theme: 'weather_one'
			}
		},
		{
			module: 'MMM-Dad-Jokes',
			position: 'top_bar',
			config: {
				updateInterval: 185001,
				filters: [
					'damn',
					'Hans free',
					'Zimbabwe'
				]
			}
		},
		{
			module: 'MMM-Wallpaper',
			position: 'fullscreen_below',
			config: {
				source: [
					'bing',
					'/r/EarthPorn',
					'/r/spaceporn',
					'/r/WaterPorn',
					'/r/AnimalPorn',
					'/r/NaturePorn',
					'/r/NatureIsFuckingLit',
					'/r/ITookAPicture',
					'/r/ArchitecturalRevival',
					'/r/naturepics',
					'/r/ruralporn',
					'/r/telephotolandscapes',
				],
				maximumEntries: 20,
				filter: 'none',
				nsfw: false
			},
			header: ''
		},
		{
			module: 'MMM-Remote-Control',
			config: {
				customCommand: {
					monitorOnCommand: 'vcgencmd display_power 1',
					monitorOffCommand: 'vcgencmd display_power 0',
					monitorStatusCommand: 'vcgencmd display_power -1 | grep -q ' +
						'"display_power=1" && echo "true" || echo ' +
						'"false"'
				},
				pm2ProcessName: "MagicMirror",
				apiKey: "XXXXXXXXX"
			}
		},
		{
			module: 'MMM-Powerwall',
			position: 'lower_third',
			config: {
				powerwallIP: 'XXXXXXXXX',
				powerwallPassword: 'XXXXXXXXX',
				teslaAPIUsername: 'XXXXXXXXX',
				teslaAPIPassword: 'XXXXXXXXX',
				home: [
					XXXXXXXXX,
					-XXXXXXXXX
				],
				twcManagerIP: 'XXXXXXXXX',
				teslamate: {
					url: "mqtt://XXXXXXXXX"
				},
				debug: true
			}
		},
		{
			module: 'MMM-WatchDog'
		}
	]

};

/*************** DO NOT EDIT THE LINE BELOW ***************/
if (typeof module !== "undefined") { module.exports = config; }

Additional Notes: The second emoji that appears in the screenshot is part of the event name.

@khassel
Copy link
Collaborator

khassel commented Feb 26, 2023

cannot reproduce this with this calendar config:

                {
                        module: "calendar",
                        header: "German Holidays",
                        position: "top_left",
                        config: {
                                fadePoint: 0.4,
                                hideTime: true,
                                showTimeToday: true,
                                maximumNumberOfDays: 90,
                                maximumEntries: 8,
                                displaySymbol: true,
                                colored: true,
                                coloredSymbolOnly: true,
                                wrapEvents: true,
                                timeFormat: "relative",
                                nextDaysRelative: true,
                                hideOngoing: false,
                                getRelative: 0,
                                calendars: [
                                        {
                                                symbol: 'user-friends',
                                                color: '#fff380',
                                                url: "https://calendar.google.com/calendar/ical/de.german%23holiday%40group.v.calendar.google.com/public/basic.ics"
                                        }
                                ]
                        }
                },

grafik

Did you change css via custom.css?

@MikeBishop
Copy link
Contributor Author

I'll grab the CSS when I'm at a keyboard, but in your repro attempt the event name wraps and the date string doesn't. In what I'm seeing, the date string is wrapped and the event name isn't. (Note the second line, which matches yours.)

@khassel
Copy link
Collaborator

khassel commented Feb 28, 2023

Can now reproduce with this ics:

BEGIN:VCALENDAR
PRODID:-//Google Inc//Google Calendar 70.9054//EN
VERSION:2.0
CALSCALE:GREGORIAN
METHOD:PUBLISH
X-WR-CALNAME:xxx@gmail.com
X-WR-TIMEZONE:Europe/Zurich
BEGIN:VTIMEZONE
TZID:Etc/UTC
X-LIC-LOCATION:Etc/UTC
BEGIN:STANDARD
TZOFFSETFROM:+0000
TZOFFSETTO:+0000
TZNAME:GMT
DTSTART:19700101T000000
END:STANDARD
END:VTIMEZONE
BEGIN:VEVENT
DTSTART;VALUE=DATE:20230227
DTEND;VALUE=DATE:20230313
DTSTAMP:20210421T154106Z
UID:zzz@google.com
REATED:20200831T200244Z
DESCRIPTION:
LAST-MODIFIED:20200831T200244Z
LOCATION:
SEQUENCE:0
STATUS:CONFIRMED
SUMMARY:Birthday
TRANSP:OPAQUE
BEGIN:VALARM
ACTION:DISPLAY
DESCRIPTION:This is an event reminder
TRIGGER:-P0DT7H0M0S
END:VALARM
END:VEVENT

@khassel khassel closed this as completed Feb 28, 2023
@khassel khassel reopened this Feb 28, 2023
@MikeBishop
Copy link
Contributor Author

Here's my custom.css:

.module:not(.MMM-WeatherOrNot),
.MMM-Wallpaper .title {
  background-color: rgba(0, 0, 0, 0.5);
  border-radius: 8px;
  padding: 8px;
}

.MMM-Wallpaper .title {
  bottom: 10px;
}

.region.top {
  top: -40px;
}

.region.left {
  max-width: 40%
}

.module.calendar_monthly {
  min-width: 300px;
}

iframe.iframe {
  width: 100%;
  max-height: 100px;
}

.region.lower.third {
  top: 62%;
}

.uptimerobot .friendlyName {
  text-align: left;
}

I don't see anything there I would expect to affect this. It looks like it happens when the date wraps, and the default rendering of a single-day event doesn't do that very often.

@sdetweil
Copy link
Collaborator

the calendar content is rows and columns in and html table

how does tr/td handle wrap?

@khassel
Copy link
Collaborator

khassel commented Feb 28, 2023

grafik

grafik

grafik

Seems to be an edge case when the width is very small, I thinks its caused by the default calendar css but I'm not good in css stuff ...

@sdetweil
Copy link
Collaborator

the row (tr)expands to wrap the largest element in the table. default td centering is centered. horizontal & vertical
td does not left/right expand, but wraps text
symbol is forced up

@khassel
Copy link
Collaborator

khassel commented Feb 28, 2023

this can be fixed by adding

.calendar .table {
  vertical-align: top;
}

in the custom.css file.

I'm not sure if this should be done in calendar.css because I don't know if this would have side effects on other calendar setups.

@sdetweil
Copy link
Collaborator

nice. think it should be custom.css

@MikeBishop
Copy link
Contributor Author

While I'm certainly happy to fix my edge-case locally, it seems unfortunate to leave a bug in because it's difficult to do CSS targeting. Is that an existing concern with the calendar module, that CSS changes can impact other modules by accident?

@khassel
Copy link
Collaborator

khassel commented Feb 28, 2023

didn't mean other modules but may others are happy with the horizontal centering. And may there is another reason that this is the default. If there is a majority to do this change in calender.css I can make a PR.

So @sdetweil @rejas whats your opinion?

@MikeBishop
Copy link
Contributor Author

didn't mean other modules but may others are happy with the horizontal centering. And may there is another reason that this is the default. If there is a majority to do this change in calender.css I can make a PR.

I'd be perfectly happy with center-alignment, too, so long as they're all the same. The weird part is two columns aligning top and one aligning center; I think either way looks fine if they're consistent.

@sdetweil
Copy link
Collaborator

the time column is not aligning top, what u see is a side affect of the need to wrap the text. which centers the wrapped result vertically

if this is a bug, it's the symbol alignment

@khassel
Copy link
Collaborator

khassel commented Feb 28, 2023

removing the first 3 lines (display, flex-direction, justify-content) in this section in calendar.css seems to solve this:

.calendar .symbol {
  display: flex;
  flex-direction: row;
  justify-content: flex-end;
  padding-left: 0;
  padding-right: 10px;
  font-size: var(--font-size-small);
}

@khassel
Copy link
Collaborator

khassel commented Feb 28, 2023

maybe @rejas remembers why he introduced this

grafik

@MikeBishop
Copy link
Contributor Author

the time column is not aligning top, what u see is a side affect of the need to wrap the text. which centers the wrapped result vertically

I could certainly be using the wrong terms, but if you look at the case where the event name wraps and the time doesn't, the time string (and the icon) aligns with the first line of the wrapped event name.

@sdetweil
Copy link
Collaborator

@MikeBishop just an accident of the table expansion
the tallest column wins

@rejas
Copy link
Collaborator

rejas commented Mar 1, 2023

maybe @rejas remembers why he introduced this

most probably because vertical-align: top is such an old css and flex the way to position element these days (and grid too but for that I'd have to check the support with our browsers).

anyway, I'd go for more flex alignements in the normal css to have a general good look in our calendar.

@khassel
Copy link
Collaborator

khassel commented Mar 1, 2023

thanks, so I looked where the vertical-align: top was introduced and found this PR.

Here one picture from this PR:
with_wrap

The Icon was set to vertical-align: top but not the other columns as you can see in the time column. The time columns was set to top in a following commit so at the moment only the title colum is not.

So I think we should align all columns to top which would fix this issue.

@CarJem
Copy link
Contributor

CarJem commented Mar 2, 2023

hope my accepted #3033 pr doesn't break things further...

@rejas
Copy link
Collaborator

rejas commented Mar 6, 2023

hope my accepted #3033 pr doesn't break things further...

I think your PR did just fine :-)

rejas pushed a commit that referenced this issue Mar 6, 2023
The title column was vertical centered before.

Fixes #3053
@looolz
Copy link

looolz commented Mar 29, 2023

I'm having an issue where the maxTitleLength: '29', isn't respected. Doesn't matter what number I put in there. The full title length is displayed. Anyone have any ideas?

Here the max title length displayed is more than 29 characters, making the entire mirror misalign.

image

`

{
module: 'calendar',
header: '',
position: 'top_left',
config: {
maximumEntries: '12',
maximumNumberOfDays: '31',
maxTitleLength: '29',
hidePrivate: true,
fade: false,
displaySymbol: false,
calendars: [
{
symbol: 'calendar-check-o ',
url: 'https://calendar.google.com/calendar/ical/URLURLURL/basic.ics'
},
{
symbol: 'cog ',
url: 'https://racingnews365.com/ics/download/calendar-formula-2023.ics'
}

  		]
  	}
  },

`

@sdetweil
Copy link
Collaborator

please remove the quotes from the numeric values.

maximumEntries: '12',
maximumNumberOfDays: '31',
maxTitleLength: '29',

numbers should not have quotes,
true/false should not have quotes
if it has a letter or symbol in the value, then it needs quotes

MichMich added a commit that referenced this issue Apr 4, 2023
## [2.23.0] - 2023-04-04

Thanks to: @angeldeejay, @buxxi, @CarJem, @dariom, @DaveChild, @dWoolridge, @grenagit, @Hirschberger, @KristjanESPERANTO, @MagMar94, @naveensrinivasan, @nfogal, @psieg, @rajniszp, @retroflex, @SkySails and @tomzt.

Special thanks to @khassel, @rejas and @sdetweil for taking over most (if not all) of the work on this release as project collaborators. This version would not be there without their effort. Thank you guys! You are awesome!

### Added

- Added increments for hourly forecasts in weather module (#2996)
- Added tests for hourly weather forecast
- Added possibility to ignore MagicMirror repo in updatenotification module
- Added Pirate Weather as new weather provider (#3005)
- Added possibility to use your own templates in Alert module
- Added error message if `<modulename>.js` file is missing in module folder to get a hint in the logs (#2403)
- Added possibility to use environment variables in `config.js` (#1756)
- Added option `pastDaysCount` to default calendar module to control of how many days past events should be displayed
- Added thai language to alert module
- Added option `sendNotifications` in clock module (#3056)

### Removed

- Removed darksky weather provider
- Removed unneeded (and unwanted) '.' after the year in calendar repeatingCountTitle (#2896)

### Updated

- Use develop as target branch for dependabot
- Update issue template, contributing doc and sample config
- The weather modules clearly separates precipitation amount and probability (risk of rain/snow)
  - This requires all providers that only supports probability to change the config from `showPrecipitationAmount` to `showPrecipitationProbability`.
- Update tests for weather and calendar module
- Changed updatenotification module for MagicMirror repo only: Send only notifications for `master` if there is a tag on a newer commit
- Update dates in Calendar widgets every minute
- Cleanup jest coverage for patches
- Update `stylelint` dependencies, switch to `stylelint-config-standard` and handle `stylelint` issues, update `main.css` matching new rules
- Update Eslint config, add new rule and handle issue
- Convert lots of callbacks to async/await
- Revise require imports (#3071 and #3072)

### Fixed

- Fix wrong day labels in envcanada forecast (#2987)
- Fix for missing default class name prefix for customEvents in calendar
- Fix electron flashing white screen on startup (#1919)
- Fix weathergov provider hourly forecast (#3008)
- Fix message display with HTML code into alert module (#2828)
- Fix typo in french translation
- Yr wind direction is no longer inverted
- Fix async node_helper stopping electron start (#2487)
- The wind direction arrow now points in the direction the wind is flowing, not into the wind (#3019)
- Fix precipitation css styles and rounding value
- Fix wrong vertical alignment of calendar title column when wrapEvents is true (#3053)
- Fix empty news feed stopping the reload forever
- Fix e2e tests (failed after async changes) by running calendar and newsfeed tests last
- Lint: Use template literals instead of string concatenation
- Fix default alert module to render HTML for title and message
- Fix Open-Meteo wind speed units
@rejas
Copy link
Collaborator

rejas commented Apr 4, 2023

I'm having an issue where the maxTitleLength: '29', isn't respected. Doesn't matter what number I put in there. The full title length is displayed. Anyone have any ideas?

Please open a new issue if this is still a problem for you @looolz

@rejas rejas closed this as completed Apr 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants