BreadcrumbsBundle icon indicating copy to clipboard operation
BreadcrumbsBundle copied to clipboard

Add the possibility to change the domain translation directly in all methods (not only config.yml)

Open Ronan-Lenor opened this issue 10 years ago • 7 comments

Before you can only do:

$breadcrumbs->addItem("welcome", '/', array() );

but now you can change the domain like that:

$breadcrumbs->addItem("welcome", '/', array(), 'admin');

Usefull when you need to change it depending on wich side of the website you are. Like a front office and back office.

Ronan-Lenor avatar Aug 29 '15 12:08 Ronan-Lenor

Partially resolve #66

bocharsky-bw avatar Aug 29 '15 22:08 bocharsky-bw

Btw, you could use translator directly in controller and add to breadcrumbs already translated text. I think this is the most right way.

IMO, translating should do translator service, not the breadcrumbs. So I don't like feature to translating breadcrumbs by itself.

But I'm also interesting to hear guys from White October.

bocharsky-bw avatar Sep 03 '15 07:09 bocharsky-bw

i corrected that "not empty" as you are right. but how about the "$firstPosition = -1" ? do you mean that it will break the BC if someone update?

i don't think that it make a lot of sense to put something between the translator's parameters and the domain. Actually the BC follow the same logic than the translator. And using the translator service inside the BC(controller) while this same translator function is REused in twig... it doesn't sound logical for me haha.

Ronan-Lenor avatar Sep 03 '15 11:09 Ronan-Lenor

@Ronan-Lenor What about backward compatibility (BC) break: the problem is you should keep previous arguments order. If anybody already use this library and pass $firstPosition value as a sixth argument to addObjectTree method, after merging your PR he has a problem, because sixth argument already $domain in your case.

So you should place $domain after all existing argumets to avoid break a BC.

bocharsky-bw avatar Sep 03 '15 12:09 bocharsky-bw

oh yeah, i was thinking about this problem too. And as i mentionned, i prefer follow the same logic than the translator. doing something different would bring this BC to a mess just to satisfy some "not aware" developers.

Pass the BC as a V2 on packagist would be also logical in this case. And if they still update on the dev-master ... well haha they will learn that this "composer" version function is here for a reason :D

Me too i would be interrested to hear about from White October

Ronan-Lenor avatar Sep 03 '15 12:09 Ronan-Lenor

I'll have a look at this over the weekend...

richsage avatar Sep 30 '15 11:09 richsage

Thanks for waiting for me on this one - and thanks for your work! At the moment I don't think we're ready for a v2 (we probably need to decide what else is important to be added for v2), but I'll keep this open and tag it accordingly :-)

richsage avatar Dec 01 '15 14:12 richsage