getkirby / kirby

Kirby's core application folder
https://getkirby.com
Other
1.32k stars 168 forks source link

[3.6.0-alpha.4] Panel areas label, title, breadcrumb translations #3626

Closed afbora closed 3 years ago

afbora commented 3 years ago

Describe the bug

Need to translate for some areas attributes like following:

Kirby::plugin('yourname/todos', [
    'areas' => [
        'todos' => function () {
            return [
                'label' => [
                    'en' => 'Todos',
                    'tr' => 'Yapılacaklar',
                ],
                'breadcrumbLabel' => function () {
                    return [
                        'en' => 'Todos',
                        'tr' => 'Yapılacaklar',
                    ];
                },
                'views' => [
                    [
                        'pattern' => 'todos',
                        'action'  => function () {
                            return [
                                'title' => [
                                    'en' => 'Todos',
                                    'tr' => 'Yapılacaklar',
                                ],
                                'breadcrumb' => function () {
                                    return [
                                        [
                                            'label' => [
                                                'en' => 'Buy some milk',
                                                'tr' => 'Biraz süt satın al',
                                            ],
                                            'link'  => '/todos/123'
                                        ]
                                    ];
                                },
                            ];
                        }
                    ]
                ]
            ];
        }
    ]
]);

Kirby Version

3.6.0-alpha.4

distantnative commented 3 years ago

I think we need to add I18n::translate() for this in a few places. Let's collect:

Not too sure yet how to handle those options with callback functions. For these, maybe the developer would need to already use I18n::translate() inside the callback function.

afbora commented 3 years ago

Yes, currently I18n::translate() method doesn't support callback.

distantnative commented 3 years ago

It would also be not efficient. We use callbacks in these places to only resolve them once needed. If I18n::translate() resolves them right away, that benefit is wasted. So I think for all callbacks, the developers have to take care of it themselves inside the closure.

afbora commented 3 years ago

You mean, developers should return array directly, instead array in closure like below example? (Of course after implements I18n::translate() to areas)

Correct

'breadcrumbLabel' => [
    'en' => 'Todos',
    'tr' => 'Yapılacaklar',
]

Wrong

'breadcrumbLabel' => function () {
    return [
        'en' => 'Todos',
        'tr' => 'Yapılacaklar',
    ];
},
distantnative commented 3 years ago

No, I think we need to keep closures for better performance - that's why Bastian implemented them in the first place.

What I mean is this:

'breadcrumbLabel' => function () {
    return I18n::translate([
        'en' => 'Todos',
        'tr' => 'Yapılacaklar',
    ]);
},
bastianallgeier commented 3 years ago

I wonder if we should really encourage the translation arrays here at all, or try to get developers to use t() instead and register the matching translation strings.

afbora commented 3 years ago

Can we translate arrays inside the Closure? We can leave that to the developer if it makes it complicated.

bastianallgeier commented 3 years ago

We use t() for all our stuff and I would think it's the most straight forward way for plugin developers as well.

A plugin could look something like this:

Kirby::plugin('your/plugin', [
    'translations' => [
        'en' => [
            'todo' => 'Todo',
        ],
        'de' => [
            'todo' => 'Aufgabe',
        ]
    ],
    'areas' => [
        'todos' => function () {
            return [
                'label' => t('todo),
                ...
            ];
        }
    ]   
]);
afbora commented 3 years ago

In this case, the developer can translate in 2 different ways. With I18n::translate() and t(). I don't think any further development is needed for this, right?

bastianallgeier commented 3 years ago

Turns out t() is just a shortcut for I18n::translate() 🤣

So this should be working as well:

'label' => t([
    'en' => 'Todos',
    'tr' => 'Yapılacaklar',
])
distantnative commented 3 years ago

I'd suggest to close this issue and make a note for when we write the docs to include an example.

bastianallgeier commented 3 years ago

I've added it to the docs: https://getkirby.com/releases/3.6/fiber/areas#translating-labels-titles-etc