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

com_fields: allowing custom field Label to be translated by a string #12656

Merged
merged 2 commits into from
Nov 13, 2016

Conversation

infograf768
Copy link
Member

@infograf768 infograf768 commented Oct 31, 2016

This patch allows the custom field label to be translated with a new string.
To test, enter a constant compatible in the Label field when you create your custom field.

Note: YES, NO, TRUE, FALSE are reserved words in INI format. ; Double quotes in the values have to be formatted as "_QQ_"

For example: JM_MYFIELD
Then add an override for the language in the Language Manager->Overrides
JM_MYFIELD="A nice demo field"
Display the field

@andrepereiradasilva
Copy link
Contributor

works as described.
But, shouldn't the same be done to other parts. e.g.: the descriptiom, hint placeholder, etc (dependng on the field)

@infograf768
Copy link
Member Author

maybe, but more complex.
Little by little :)

Note : for this PR, maybe we should check the $label before using JText::_(), i.e. make sure its formatted correctly with a regex.

@andrepereiradasilva
Copy link
Contributor

isn't there a JField attribute to allow translations? i think i saw that somewhwere

@infograf768
Copy link
Member Author

yes, but this would mean adding these in the custom field in a way or another as a choice by the user. i.e new columns i guess in the table

protected $translateLabel = true;

@infograf768
Copy link
Member Author

In fact, this PR is absolutely not enough. This is only used in frontend.
We do get there by using in render.php

var_dump($field->description);
var_dump($field->params['hint']);

string 'mydescription' (length=13)

string 'ahint' (length=5)

and when editing and debugging
screen shot 2016-11-01 at 08 40 26

@laoneo
Copy link
Member

laoneo commented Nov 1, 2016

Where else do you need it?

@infograf768
Copy link
Member Author

infograf768 commented Nov 1, 2016

Tested further

When adding the strings in en-GB.override.ini both in frontend and backend, I do get the forms translated
JMFIELD="My field"
AHINT="Test a translation of hint"
MYDESCRIPTION="Test a translatable description"

screen shot 2016-11-01 at 04 10 19
Remark: The Fields tab is double translated
Which is not the case when I use a Field Group AND add a new string
screen shot 2016-11-01 at 04 15 41
with
CONTENT-GROUP_1="Custom fields 1"

To solve the double translation for $key = 'JGLOBAL_FIELDS'; I guess some more owrk has to be done lines 385-410 of ROOT/administrator/components/com_fields/helpers/fields.php

Therefore concerning that aspect, this PR is fine for articles as it adds the translated label in the article info.
But will we always need to display the label in some other cases (when not editing a form)? I guess it is should be possible to choose to only display the value.
In that case the label should be hidden and only the value displayed.
What do you think?


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/12656.

@laoneo
Copy link
Member

laoneo commented Nov 2, 2016

As almost all code is getting the fields trough this function you can add the JText call of the label somewhere here https://github.com/joomla/joomla-cms/blob/staging/administrator/components/com_fields/helpers/fields.php#L102. Then you have it translated everywhere and not only during the output.

@infograf768
Copy link
Member Author

@laoneo
It is by default translated in the forms. It is in the display that it is not.

@laoneo
Copy link
Member

laoneo commented Nov 7, 2016

Yeps, but when you use the fields directly (they are attached to the item/article as a new variable fields) on a template override, then the label is not translated. Do we want that? If yes, then all is ok here.

@Bakual
Copy link
Contributor

Bakual commented Nov 7, 2016

I have tested this item ✅ successfully on 4820b33

Frontend now displays the translated label.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/12656.

@infograf768
Copy link
Member Author

Yeps, but when you use the fields directly (they are attached to the item/article as a new variable fields) on a template override, then the label is not translated. Do we want that? If yes, then all is ok here.

not sure what you mean. i would need a concrete example i can reproduce easily (code included)

@laoneo
Copy link
Member

laoneo commented Nov 7, 2016

In the file components/com_content/views/article/tmpl/default.php you can do print_r($this->item->fields);. It will print the fields. Here are the labels not translated. That was what I was talking about.

@infograf768
Copy link
Member Author

print_r($this->item->fields); will just not work anyway (Gets the details of the array) and doing an echo instead of print_r will create the array to string conversion error.

let's get this PR merged.

@ggppdk
Copy link
Contributor

ggppdk commented Nov 8, 2016

I have tested this item ✅ successfully on 4820b33


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/12656.

@infograf768
Copy link
Member Author

RTC. Thanks for testing.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/12656.

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Nov 8, 2016
@roland-d roland-d added this to the Joomla 3.7.0 milestone Nov 13, 2016
@roland-d roland-d merged commit 1a21f17 into joomla:staging Nov 13, 2016
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Nov 13, 2016
@infograf768 infograf768 deleted the com_fields_patch_3 branch November 13, 2016 06:28
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.

7 participants