Skip to content

Added support for optional DEGREE position in "FEELS" label#2395

Merged
MichMich merged 2 commits intoMagicMirrorOrg:developfrom
ashishtank:develop
Dec 31, 2020
Merged

Added support for optional DEGREE position in "FEELS" label#2395
MichMich merged 2 commits intoMagicMirrorOrg:developfrom
ashishtank:develop

Conversation

@ashishtank
Copy link
Contributor

In some languages the value (number) should be first and then sentence. Added support for Degree label so it shows correct translation for Hindi and Gujarati with degree label at first and not at last.

@MichMich MichMich merged commit 87e2e87 into MagicMirrorOrg:develop Dec 31, 2020
@rejas
Copy link
Collaborator

rejas commented Dec 31, 2020

Please revert @MichMich or please @ashishtank fix this issue that got introduced: Using the weather (and not the currentweather-) module the translation looks like this:

grafik

Porblem is both modules use the same translation but not the same logic (weaather is doing it in NJK file)

@ashishtank
Copy link
Contributor Author

@rejas let me check

@ashishtank
Copy link
Contributor Author

@MichMich @rejas I could not find a way to pass value to translate. If you know how to do that please let me know else revert the pull request (or may be keep it as it only affect the languages with {DEGREE} placeholder ). I will try to fix it other way.

@ashishtank
Copy link
Contributor Author

Looks like in our custom translate filter we do not have provision to pass key and values ?
image

@rejas
Copy link
Collaborator

rejas commented Dec 31, 2020

Indeed. So maybe add some params to it like the filters in weather.js have?

@ashishtank
Copy link
Contributor Author

Yes, I have solved it.. doing final testing.

@MichMich
Copy link
Collaborator

Ah. I’ll wait revering it.

@MichMich
Copy link
Collaborator

*reverting

@ashishtank
Copy link
Contributor Author

Finally solved it #2397, sending updated PR. Also added support for placeholder values for translate in nunjucks translate filter.

English
image

Gujarati
image

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