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

make info boxes for posts translatable #484

Merged
merged 8 commits into from
Mar 4, 2020
Merged

Conversation

andibraeu
Copy link
Contributor

By default there was a line "Posted in on <Month day, Year>" for every language.

This PR enables users to use the translation plugin to translate these strings.

@bennothommo
Copy link
Contributor

@andibraeu Would it be better to use the language strings directly in the component partial, instead of loading them up with the model? You can see an example here: rainlab/user-plugin#402 - which is a similar change to the one you are proposing, but for the User plugin.

@bennothommo bennothommo added Status: Response Needed Requires / is waiting on a response from at least one of the participants Type: Maintenance Minor maintenance to the code base (i.e. minor bug fixes, styling fixes, translation improvements) labels Nov 10, 2019
@andibraeu
Copy link
Contributor Author

thanks for your feedback.

Unfortunately I didn't get it to work with default values then.

rainlab.blog::lang.post.posted_on_prefix will be shown instead of a default value.

@LukeTowers
Copy link
Contributor

@andibraeu why do you need default values? What is being passed is a backend language string, so use the filter that's meant to handle those (| trans, not | _). I.e. {{ 'rainlab.blog::lang.post.posted_on_prefix' | trans }}

@andibraeu
Copy link
Contributor Author

thanks for your help, I'm new to twig and octobercms :)

I've made the changes

components/Posts.php Outdated Show resolved Hide resolved
@bennothommo bennothommo added Status: Revision Needed Requires changes before it can be accepted and removed Status: Response Needed Requires / is waiting on a response from at least one of the participants labels Nov 14, 2019
@bennothommo bennothommo self-assigned this Nov 14, 2019
@andibraeu
Copy link
Contributor Author

any news on that?

@bennothommo
Copy link
Contributor

@andibraeu Would you mind testing my changes and let me know if that works for you? Also, feel free to adjust the lang file if necessary - wasn't sure if that's the correct syntax.

@LukeTowers let me know if my changes were what you were after.

@bennothommo bennothommo added Status: Review Needed Requires review from a maintainer or trusted community member and removed Status: Revision Needed Requires changes before it can be accepted labels Dec 3, 2019
@LukeTowers
Copy link
Contributor

LukeTowers commented Dec 3, 2019

@bennothommo that looks good to me but can we have spaces around the Twig filter bar? i.e. val | filter instead of just val|filter? In my opinion that makes it much easier to read.

@andibraeu
Copy link
Contributor Author

That change means, that we'll see "in uncategorized" for every post that's not in a catergory?

@LukeTowers
Copy link
Contributor

Good catch @andibraeu. @bennothommo I think we should remove the uncategorized fallback

@LukeTowers
Copy link
Contributor

Although not sure how to best handle that

@bennothommo
Copy link
Contributor

@LukeTowers I did consider that as well, but I didn't think any of the options for that were graceful if we were attempting to simplify the wording down to one language string, ie.

  • Have a second language string if no categories are provided.
  • Adding an "in " prefix to the categoryLinks variable above if categories are present, and remove it from the language string.

I felt having it show up as Uncategorized fits the spirit of the plugin, given that Blog posts are categorised, and matches similar functionality to Wordpress. If people didn't like it, they always had the option of overwriting the partial.

@LukeTowers
Copy link
Contributor

@bennothommo I agree with you in principle, however this is a change to the default partial that people are using on their sites out in the wild. You wouldn't expect random strings to start appearing on your frontend after an update so we can't really make that sort of change here.

@bennothommo
Copy link
Contributor

@LukeTowers Fair point - I've added a second language string when no categories are present, so the difference in languages can be done purely through the language files. Also, added spaces around the Twig filters.

@andibraeu
Copy link
Contributor Author

are there any news on that?

@bennothommo
Copy link
Contributor

@andibraeu The changes are awaiting confirmation that they work before being merged. Have you had a chance to test them?

@andibraeu
Copy link
Contributor Author

yes, I tested it, and I use it. You can see the results here: https://weimarnetz.de/blog

@bennothommo bennothommo added this to the v1.4.1 milestone Mar 4, 2020
@bennothommo bennothommo added Status: Completed Fix is completed and merged into the dev branch; remaining open until it has been merged into master and removed Status: Review Needed Requires review from a maintainer or trusted community member labels Mar 4, 2020
@bennothommo
Copy link
Contributor

Thanks @andibraeu!

@bennothommo bennothommo merged commit bd99d45 into rainlab:master Mar 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Completed Fix is completed and merged into the dev branch; remaining open until it has been merged into master Type: Maintenance Minor maintenance to the code base (i.e. minor bug fixes, styling fixes, translation improvements)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants