Skip to content

Conversation

@bughaver
Copy link
Contributor

@bughaver bughaver commented May 9, 2025

Hello and thank you for wanting to contribute to the MagicMirror² project!

Please make sure that you have followed these 4 rules before submitting your Pull Request:

  1. Base your pull requests against the develop branch.
  2. Include these infos in the description: implement short syntax for clock week
  • Does the pull request solve a related issue? n/a
  • If so, can you reference the issue like this Fixes #<issue_number>?
  • What does the pull request accomplish? Use a list if needed.
  • If it includes major visual changes please add screenshots.
  1. Please run npm run lint:prettier before submitting so that
    style issues are fixed.
  2. Don't forget to add an entry about your changes to
    the CHANGELOG.md file.

image

image

Note: Sometimes the development moves very fast. It is highly
recommended that you update your branch of develop before creating a
pull request to send us your changes. This makes everyone's lives
easier (including yours) and helps us out on the development team.

Thanks again and have a nice day!

@bughaver
Copy link
Contributor Author

bughaver commented May 9, 2025

const weekWrapperInnerHTML = this.translate("WEEK", { weekNumber: now.week() });

if (this.config.showWeek === "short") {
weekWrapper.innerHTML = weekWrapperInnerHTML.replace("Week ", "W");
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this won't work for the translated "week" in other languages...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated the implementation to support all translations

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldnt it be much simpler to add a New translation string like WEEK_SHORT? any string manipulation fails if a language has the number first and the week string second...

Copy link
Contributor Author

@bughaver bughaver May 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your review @rejas! will look into how to implement WEEK_SHORT

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rejas updates all the translations using WEEK and maintained order of language. I used ai to help as I could only wish to be fluent in every language.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Personally I don't like using AI but rely on real life speakers. Don't know how the othera feel about this but even if they would agree I would want to have that kind of "ai translations" in a seperate PR

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've removed the AI translations

@bughaver bughaver force-pushed the clock-week branch 2 times, most recently from c37a01a to f4bdff0 Compare May 10, 2025 04:40
"RUNNING": "noch",
"EMPTY": "Keine Termine.",
"WEEK": "{weekNumber}. Kalenderwoche",
"WEEK_SHORT": "{weekNumber}K",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a German I know that we would shorten it as "KW", so AI is wrong here already...

Copy link
Contributor

@bugsounet bugsounet May 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps the best choice is to translate WEEK_SHORT only into languages ​​we are sure of.
For other languages, we will make another PR.

In all case if there is no translate, it will fallback to default language

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rejas oh that's my bad.. it did generate it as KW but I was changed it to a single character. I can change it back.

Copy link
Contributor Author

@bughaver bughaver May 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rejas @bugsounet implemented as you suggested

@bugsounet
Copy link
Contributor

If you want, you can add in translations/fr.json: "WEEK_SHORT": "S{weekNumber}", (my native language)

Thx

@bughaver
Copy link
Contributor Author

If you want, you can add in translations/fr.json: "WEEK_SHORT": "S{weekNumber}", (my native language)

Thx

Pushed changes!

@bughaver
Copy link
Contributor Author

@rejas @bugsounet pr should be ready for your final review

@rejas rejas merged commit 2f9f4b6 into MagicMirrorOrg:develop May 16, 2025
9 checks passed
rejas pushed a commit that referenced this pull request Jun 5, 2025
**Short description**: I completed the translations with the help of
translation tools.

**Long description**: I'm currently looking at the translation-tests and
I noticed the e2e-test `${language} should contain all base keys, ()`.
It is supposed to check whether all keys are present in each translation
file, but it doesn't actually work that way. When I fix it, a lot of
missing translations are reported. I have completed these translations
with the help of translation tools and packed them into this PR. I have
left out the modified test so that we can focus on the question if we
accept such amount of automatic translations or not.

rejas has already expressed in another PR
(#3775 (comment))
that he prefers human translators. I basically do too, but I don't see
how we can manage to have all translations completed by humans. And even
if a few translations are not correct, hopefully a user will get in
touch.

If we decide against the translations, we should at least remove the
test. If we go for the tranlsations, I'll add the test.

What do you think?

---------

Co-authored-by: Magnus <34011212+MagMar94@users.noreply.github.com>
@khassel khassel mentioned this pull request Jun 30, 2025
khassel added a commit that referenced this pull request Jun 30, 2025
## [2.32.0] - 2025-07-01

Thanks to: @bughaver, @bugsounet, @khassel, @KristjanESPERANTO,
@plebcity, @rejas, @sdetweil.

> ⚠️ This release needs nodejs version `v22.14.0 or higher`

### Added

- [config] Allow to change module order for final renderer (or
dynamically with CSS): Feature `order` in config (#3762)
- [clock] Added option 'disableNextEvent' to hide next sun event (#3769)
- [clock] Implement short syntax for clock week (#3775)

### Changed

- [refactor] Simplify module loading process (#3766)
- Use `node --run` instead of `npm run` (#3764) and adapt `start:dev`
script (#3773)
- [workflow] Run linter and spellcheck with LTS node version (#3767)
- [workflow] Split "Run test" step into two steps for more clarity
(#3767)
- [linter] Review linter setup (#3783)
  - Fix command to lint markdown in `CONTRIBUTING.md`
  - Re-activate JSDoc linting and fix linting issues
  - Refactor ESLint config to use `defineConfig` and `globalIgnores`
  - Replace `eslint-plugin-import` with `eslint-plugin-import-x`
- Switch Stylelint config to flat format and simplify Stylelint scripts
- [workflow] Replace Node.js version v23 with v24 (#3770)
- [refactor] Replace deprecated constants `fs.F_OK` and `fs.R_OK`
(#3789)
- [refactor] Replace `ansis` with built-in function `util.styleText`
(#3793)
- [core] Integrate stuff from `vendor` and `fonts` folders into main
`package.json`, simplifies install and maintaining dependencies (#3795,
#3805)
- [l10n] Complete translations (with the help of translation tools)
(#3794)
- [refactor] Refactored `calendarfetcherutils` in Calendar module to
handle timezones better (#3806)
  - Removed as many of the date conversions as possible
- Use `moment-timezone` when calculating recurring events, this will fix
problems from the past with offsets and DST not being handled properly
- Added some tests to test the behavior of the refactored methods to
make sure the correct event dates are returned
- [linter] Enable ESLint rule `no-console` and replace `console` with
`Log` in some files (#3810)
- [tests] Review and refactor translation tests (#3792)

### Fixed

- [fix] Handle spellcheck issues (#3783)
- [calendar] fix fullday event rrule until with timezone offset (#3781)
- [feat] Add rule `no-undef` in config file validation to fix #3785
(#3786)
- [fonts] Fix `roboto.css` to avoid error message `Unknown descriptor
'var(' in @font-face rule.` in firefox console (#3787)
- [tests] Fix and refactor e2e test `Same keys` in
`translations_spec.js` (#3809)
- [tests] Fix e2e tests newsfeed and calendar to exit without open
handles (#3817)

### Updated

- [core] Update dependencies including electron to v36 (#3774, #3788,
#3811, #3804, #3815, #3823)
- [core] Update package type to `commonjs`
- [logger] Review factory code part: use `switch/case` instead of
`if/else if` (#3812)

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: Michael Teeuw <michael@xonaymedia.nl>
Co-authored-by: Kristjan ESPERANTO <35647502+KristjanESPERANTO@users.noreply.github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Ross Younger <crazyscot@gmail.com>
Co-authored-by: Veeck <github@veeck.de>
Co-authored-by: Bugsounet - Cédric <github@bugsounet.fr>
Co-authored-by: jkriegshauser <joshuakr@nvidia.com>
Co-authored-by: illimarkangur <116028111+illimarkangur@users.noreply.github.com>
Co-authored-by: sam detweiler <sdetweil@gmail.com>
Co-authored-by: vppencilsharpener <tim.pray@gmail.com>
Co-authored-by: veeck <michael.veeck@nebenan.de>
Co-authored-by: Paranoid93 <6515818+Paranoid93@users.noreply.github.com>
Co-authored-by: Brian O'Connor <btoconnor@users.noreply.github.com>
Co-authored-by: WallysWellies <59727507+WallysWellies@users.noreply.github.com>
Co-authored-by: Jason Stieber <jrstieber@gmail.com>
Co-authored-by: jargordon <50050429+jargordon@users.noreply.github.com>
Co-authored-by: Daniel <32464403+dkallen78@users.noreply.github.com>
Co-authored-by: Ryan Williams <65094007+ryan-d-williams@users.noreply.github.com>
Co-authored-by: Panagiotis Skias <panagiotis.skias@gmail.com>
Co-authored-by: Marc Landis <dirk.rettschlag@gmail.com>
Co-authored-by: HeikoGr <20295490+HeikoGr@users.noreply.github.com>
Co-authored-by: Pedro Lamas <pedrolamas@gmail.com>
Co-authored-by: veeck <gitkraken@veeck.de>
Co-authored-by: Magnus <34011212+MagMar94@users.noreply.github.com>
Co-authored-by: Ikko Eltociear Ashimine <eltociear@gmail.com>
Co-authored-by: DevIncomin <56730075+Developer-Incoming@users.noreply.github.com>
Co-authored-by: Nathan <n8nyoung@gmail.com>
Co-authored-by: mixasgr <mixasgr@users.noreply.github.com>
Co-authored-by: Savvas Adamtziloglou <savvas-gr@greeklug.gr>
Co-authored-by: Konstantinos <geraki@gmail.com>
Co-authored-by: OWL4C <124401812+OWL4C@users.noreply.github.com>
Co-authored-by: BugHaver <43462320+bughaver@users.noreply.github.com>
Co-authored-by: BugHaver <43462320+lsaadeh@users.noreply.github.com>
Co-authored-by: Koen Konst <koenspero@gmail.com>
Co-authored-by: Koen Konst <c.h.konst@avisi.nl>
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.

3 participants