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

Labels with Nette\Utils\Html are not translated #225

Merged
merged 1 commit into from Jul 8, 2019

Conversation

MilanPala
Copy link
Contributor

@MilanPala MilanPala commented Jun 21, 2019

  • bug fix: yes
  • BC break? no

@dg
Copy link
Member

@dg dg commented Jul 7, 2019

Which bug does it solve?

@mabar
Copy link
Contributor

@mabar mabar commented Jul 7, 2019

Translator which supports only strings, I guess.

Easier would be solve it in translator, this is probably not the only place where is non-string value translated.

if (!is_string($message)) {
  return (string) $message;
}

@dg
Copy link
Member

@dg dg commented Jul 7, 2019

It creates inconsistency with others usage of translate().

@MilanPala
Copy link
Contributor Author

@MilanPala MilanPala commented Jul 8, 2019

With behavior in Nette 3 i can't use Nette\Utils\Html as label (for example with links to terms and conditions). In Nette 2 I create label with link and push it into Nette\Forms\Controls\Checkbox. It works in Nette 3 too: https://github.com/nette/forms/blob/master/src/Forms/Controls/Checkbox.php#L26 But translator converts \Nette\Utils\Html\ instance into string and escape html tags into entities: https://github.com/nette/forms/blob/master/src/Forms/Controls/BaseControl.php#L279 and https://github.com/nette/utils/blob/master/src/Utils/ITranslator.php#L22

In Nette 2 I don't change Nette\Utils\Html in my translator and it works so far:

	if ($message instanceof Nette\Utils\Html) {
			return $message;
	}

In this case would be better use Nette\Forms\Controls\BaseControl::translate instead of $this->getForm()->getTranslator()->translate() in https://github.com/nette/forms/blob/master/src/Forms/Controls/BaseControl.php#L278-L279, because this method has condition for this use case. What do you think?

@mabar
Copy link
Contributor

@mabar mabar commented Jul 8, 2019

I see. Translator must return string, but string is escaped by template engine.

@dg dg force-pushed the master branch 2 times, most recently from 608d595 to b3356d0 Compare Jul 8, 2019
@dg dg merged commit 4876961 into nette:master Jul 8, 2019
2 checks passed
@dg
Copy link
Member

@dg dg commented Jul 8, 2019

Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants