Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
Implement a generic ui for all i18n_string translations.
Comment | File | Size | Author |
---|---|---|---|
#54 | i18n-string-source-1445682-53.patch | 29.25 KB | Berdir |
#54 | i18n-string-source-1445682-53-interdiff.txt | 613 bytes | Berdir |
#51 | i18n-string-source-1445682-51.patch | 29.25 KB | Berdir |
#51 | i18n-string-source-1445682-51-interdiff.txt | 7.35 KB | Berdir |
#48 | tmgmt_i18n_string-ui-1445682-12.patch | 29.47 KB | blueminds |
Comments
Comment #1
webflo CreditAttribution: webflo commentedDepends on the patch in #1445344: Bogus static cache in i18n_object().
Comment #2
webflo CreditAttribution: webflo commentedComment #3
BerdirComment #4
BerdirDoes this still apply? I see that the blocking issue has been commited a while ago, yay!
Comment #5
BerdirYes, it applies, but it doesn't work:
Fatal error: Call to undefined function i18n_object_by_name()
I guess that function was renamed or something, i18n likes renaming functions :p
Also, integration with the translate tab would be great. You said that you have some views-based code for the overviews? That sounds interesting as well..
Comment #6
BerdirAh, yes it has been renamed to i18n_get_object() in a later patch.
When changing that then it kinda works, it lists my item types and correctly shows translated. Given that with i18n_string, the source is always the default language, I think we don't know the show either "Source" nor the language column for that, no?
Comment #7
webflo CreditAttribution: webflo commentedComment #8
BerdirWould like to see what the testbot does with this, setting to needs review. The testbot still seems to depend on 7.x-1.5, not sure why exactly, so I guess that i18n_object/i18n_get_object() conditional is still required... probably need to ask rfay for a manual dependency rebuild.
Comment #10
Berdir"Call to undefined method i18n_object_wrapper::get_strings()", I guess this depends on some uncommited i18n code?
Comment #11
inventlogic CreditAttribution: inventlogic commented#7: tmgmt_i18n_string-ui-1445682-2.patch queued for re-testing.
Comment #13
inventlogic CreditAttribution: inventlogic commentedThis is now failing on a TMGMT string test so can the patch submitter re-look at this patch?
Comment #14
miro_dietikerWe would be happy if you can take a look too. Webflo is currently very busy and the patch was also just a start.
Comment #15
BerdirAnd old, this doesn't include the work that he did in Munich. Where he, I think, changed everything again :)
Comment #16
inventlogic CreditAttribution: inventlogic commentedI was directed to this patch from Dynamic Properties issue 1700162 which said this patch would help in translating the labels for the properties.
I had a look at the patch and as it stands at the moment it does not pick up any of the Dynamic Properties label strings.
Also am I right in that I think everything that it does pick up is manually translatable some where else?
So this patch is just a strings workflow UI for TMGMT?
Its still failing on
Call to undefined method i18n_object_wrapper::strings_update()
which makes no sense as the i18n string functions are available.Anyway not what I am looking for.
Comment #17
BerdirYou are in the right issue but this patch is not yet ready. If you have a budget, I suggest you contact @webflow to sponsor the work he's doing.
Comment #18
inventlogic CreditAttribution: inventlogic commentedI figured out I can setup multiple templates in Dynamic Properties one for each language.
The translate option for the Dynamic Field can be used to fill in the language specific template.
Then its just a matter of filling in the language specific data into the template.
So sorry I wont need this patch just yet.
Comment #19
miro_dietiker@inventlogic: It's a pity to invest time to create workarounds instead of real solutions. This way, no solutions will improve ever. Good luck with your project.
Comment #20
inventlogic CreditAttribution: inventlogic commented@miro_dietiker: true and thanks.
Comment #21
miro_dietikerWe are referring here to "i18n source" which might have some special cases in addition to the general locale table.
However i guess we should start with a general locale implementation first, supporting all text groups, and then customize the i18n strings text group by leveraging i18n specific features.
Comment #22
miro_dietikerThis has been excalated to us and we will need to take care of it to make this source plugin work.
A basic version is enough... We can always improve it later!
Also note that in past, almost all projects that wanted to use TMGMT resulted in the first job to try to submit translation of menu items... It was always very frustrating to tell the clients "no, it's only content for now ... plus other entities ... almost anything, except menus / i18n strings..."
Comment #23
blueminds CreditAttribution: blueminds commentedI played with the patch and can say it works (with a few fixes) for menus, vocabularies and such. It does not work for menu links, taxonomy terms, etc... Reason is that i.e. menu links do not have records in the i18n_string but individual items are created for each language. The current code does not handle this. So what I understand that should be done:
- provide functional translation source overview page (now each lists the same content) that also can work with menu items and tax. terms with some search possibilities
- extend logic to be able to receive translation for all i18n sources
I could not find anything else that I think should be part of the basic version.
Comment #24
BerdirMenu links and taxonomy terms tepends on the localization setting on the menu/vocabulary.
I think localized is the case that we want to support here, that means that there is a single term/link with language undefined, then you have a i18n_string translation.
Other cases are not supported by us, at least not now because those usually mean that there isn't a 1-1 translation of the menu links but they differ for each language. So we can't really provide a translation anyway.
And yes, we want an overview and an integration with the translate tab, to be able to translate a single menu item without going through the overview.
Comment #25
blueminds CreditAttribution: blueminds commentedHere is patch making the basics functional. However this is not final. It needs following:
- general clean up and refactor - there is a lot of commented out code, or some unused code, needs comments..
- currently following types are implemented: menu, menu items, taxonomy vocabs, taxonomy terms. We need to decide what other types we want to implement (I guess there are others like blocks, forum, paths...) It would be also nice to provide some pluggable design so that we do not need to "if" each type
- provide tests coverage for menu items, taxonomy terms, possibly others that we will implement
Comment #26
blueminds CreditAttribution: blueminds commentedPatch includes following:
- integrated into the translate tab of i18n
- tests coverage
- cleaned code + documentation
Comment #27
BerdirQuick review, will test this soon.
Returns instead of Gets/Get
Also, we should probably talk about i18n object types as only i18n objects have a type and strings just have an arbitrary context.
Can we add a @todo here that says Support more/all i18n object tpyes?
"fixed" isn't very clear, should say what exactly you have to do, e.g. pass through function function_name(). @see should IMHO only be used at the end of a docblock, one per line. Function references within a text should just use function_name() as mentioned above.
Also > 80 chars.
state.
Hm, the #text could be quite long?
Comment #28
Berdirok, started testing this.
Working:
- Overview looks fine, search works and translation status also seems to work
- Only the group types show up that are defined, term and vocabulary for example don't if i18n_vocabulary isn't enabled.
- Creation of jobs on the translate tabs works fine.
Not working/Improvable:
- I'm not able to create jobs on the overview. Nothing happens when I click on Request translation, the page just reloads
- You're using the last data item as the source label. However, you should be using the first as that has a much higher chance to actually be a label or so for that i18n object. See attached screenshot of a job item for the management menu. The fact that the job/item labels are check_plain()'d too often isn't your problem directly nor is the fact that the job label can easily get longer than 128 in which case the form validation fails but you at least wouldn't trigger these core bugs. I think that is more or less consistent with what the translatabe tab does. So as a summary, just "#text (id)", without label, of the first data item.
- There is no source uri defined, I think that is stored in the options somewhere? The result is that it's displayed as a link to the front page. Looking at i18n_string_object_wrapper, there's get_path(), get_translate_path() and get_edit_path(), We should probably use get_path()?
- There's no paging, so if you create 1000 terms, you get a pretty large and slow page. Not sure how hard it is to add paging, if there's a problem we can postpone it.
Ideas, probably for later:
- As mentioned above, we're actually doing i18n object integration which is the part that groups several i18n_strings into a group. There are also i18n_string strings that don't belong to a group. Translatable variables are such an example I think. So I'm wondering if we should provide a separte a separate source that interacts directly with single i18n_strings's that do not belong to a known group.
Comment #29
blueminds CreditAttribution: blueminds commentedComments implemented, please see the patch.
Comment #30
blueminds CreditAttribution: blueminds commentedPrevious patch has tests disabled.
Moreover I think we need some UI tests here as well...
Comment #31
miro_dietikerPlease provide more description.
So you have implemented all "working/improvable" above?
Just "ideas, probably for later" remaining?
Comment #32
seasonxue2013 CreditAttribution: seasonxue2013 commentedWhere can I get i18n_string?
Comment #33
alanburke CreditAttribution: alanburke commentedadmin/config/regional/tmgmt/i18n_string will fail if the language "Portugese, Portugal" is on the system,
Something to do with the language code having a hyphen, I think, but I haven't worked it out yet.
Comment #34
blueminds CreditAttribution: blueminds commentedFixing the bug + providing UI test coverage.
Comment #36
blueminds CreditAttribution: blueminds commentedlooks like test bot does not like the way the job id is determined from the url...
Comment #38
miro_dietiker#36: tmgmt_i18n_string-ui-1445682-8.patch queued for re-testing.
Comment #40
blueminds CreditAttribution: blueminds commented#36: tmgmt_i18n_string-ui-1445682-8.patch queued for re-testing.
Comment #42
blueminds CreditAttribution: blueminds commentedanother try...
Comment #44
blueminds CreditAttribution: blueminds commentedand another try
Comment #46
blueminds CreditAttribution: blueminds commented...
Comment #48
blueminds CreditAttribution: blueminds commented...
Comment #50
miro_dietikerNot yet committed.. But once done, this is a starting list of followups we split from the current issue:
#2006416: i18n Strings: Support blocks
#2006422: i18n Strings: Support string record without object structure
Comment #51
BerdirHere we go, this should be ready for an initial commit I think, explanations in next comment.
Comment #52
BerdirInstead of limiting it to a few, I just excluded the broken ones, which is block and field/field instance. Everything else is working now, including node types, contact categories, rules (see below), also tested one of my contrib modules (User Relationship types).
The problem with those that don't have a type is that they don't follow the standard pattern of type:id:property, block has module:delta, so basically a combined type and fields/instances are doing stuff like field_name:bundle:something (bundle isn't limited to the entity, so I guess this would be funny if you have multiple entity types with the same bundle ;))
The function was only used in a single place, when querying and there we can actually derive this information.
The db_type is actually the string translation type and we also need to filter on the textgroup. For example, both node types (textgroup node) and user relationship types (textgroup user_relationships) use type 'type'.
Fixed a array nesting bug here that resulted in fatal errors with PHP 5.4 as it actually returned FALSE there.
Consistent way to check if the type is supported instead of relying on the (deleted) function.
This is ugly. entity.module provides generic i18n object controllers but they don't specify a load callback or entity type, i18n_string needs that to load the object behind it, no idea how it works currently, possibly it's passed in manually.
This should be fixed there but this works..
This was the main problem for rules_config, the other was an incorrect magic db_type calculation.
This caused a lot of string as string index notices in PHP 5.4, not sure how it ever worked...
This is what broke the testbot, that url() is wrong and it was then passed through url() twice.
Comment #54
BerdirUps.
Comment #55
cgalli CreditAttribution: cgalli commentedapplied patch and tested. works as described (after turning on the seperate module...)
Comment #56
BerdirCommitted and pushed then, we already have the initial follow-ups I think. Yay, that was a fight ;)