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

Remove unneeded (and unwanted) '.' after the year in calendar repeatingCountTitle #2896

Closed
mcavoya opened this issue Aug 5, 2022 · 7 comments

Comments

@mcavoya
Copy link

mcavoya commented Aug 5, 2022

Platform: Raspberry Pi 3 (browser/Electron version agnostic)
Node Version: 16.16.0
MagicMirror² Version: 2.20.0
Description: Calendar shows a '. ' after the year in repeatingCountTitle
Steps to Reproduce: Load a calendar with repeating events. e.g. Joe's birthday, with a start date seven years ago, that repeats yearly.
Expected Results: "Joe's Birthday, 7 years"
Actual Results: "Joe's Birthday, 7. years"
Configuration: see attached config.zip
Additional Notes: The fix is simple.
Change line 265 in calendar.js

from: repeatingCountTitle = ", " + yearDiff + ". " + repeatingCountTitle;
to:   repeatingCountTitle = ", " + yearDiff + repeatingCountTitle;

In cases where puncutation is desired after yearDiff, the config file can simply add it in the repeatingCountTitle.

@rejas
Copy link
Collaborator

rejas commented Aug 9, 2022

Thx for the report @mcavoya It indeed seems a little off when looking at the result in english.
Since @MichMich wrote that line (way back in 2016) maybe he had some other i18n format in mind?

@khassel
Copy link
Collaborator

khassel commented Jan 13, 2023

I think this should be fixed if there comes no veto by @MichMich

@khassel khassel added the bug label Jan 13, 2023
@MichMich
Copy link
Collaborator

All fine by me. Not emotionally attached to the .. ;)

@khassel khassel self-assigned this Jan 14, 2023
rejas pushed a commit that referenced this issue Jan 14, 2023
- removed unneeded (and unwanted) '.' after the year in calendar
repeatingCountTitle (fixes #2896)
- update Collaboration.md
@rejas rejas added this to the 2.23 milestone Feb 18, 2023
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
@khassel khassel closed this as completed Apr 4, 2023
@mcavoya
Copy link
Author

mcavoya commented Apr 7, 2023

I do not believe this topic should be marked Closed.

The code was refactored, but the offending '.' character remains. It appears right after {yearDiff}.

repeatingCountTitle = ', ${yearDiff}. ${repeatingCountTitle}';

And it can be seen here as "31. Years"

Screenshot 2023-04-07 070457

@khassel
Copy link
Collaborator

khassel commented Apr 7, 2023

you are right, sadly it was reintroduced by another PR:

grafik

So I will reopen this issue and fix it again with a new PR ...

@khassel khassel reopened this Apr 7, 2023
khassel added a commit to khassel/MagicMirror that referenced this issue Apr 7, 2023
@mcavoya
Copy link
Author

mcavoya commented Apr 7, 2023

Thanks. I can only imagine that's a frequent squence of events.

rejas pushed a commit that referenced this issue Apr 7, 2023
after the year in calendar repeatingCountTitle (#2896, second attempt
...)
@khassel
Copy link
Collaborator

khassel commented Jul 1, 2023

fixed with new release v2.24.0

@khassel khassel closed this as completed Jul 1, 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

4 participants