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

[RFC] Allow empty page titles #5545

Closed
kiler129 opened this issue Dec 25, 2022 · 1 comment · Fixed by #5549
Closed

[RFC] Allow empty page titles #5545

kiler129 opened this issue Dec 25, 2022 · 1 comment · Fixed by #5549
Milestone

Comments

@kiler129
Copy link
Contributor

kiler129 commented Dec 25, 2022

Short description of what this feature will allow to do:
In the original implementation the CrudDto::getCustomPageTitle() returned empty strings when empty strings were set via Crud::setPageTitle($pageName, ''). This behavior changed as a part of #5066 and empty() check was introduced, which later on became an explicit check for an empty string or null.

While this behavior makes sense in terms of null, to provide default values automatically, the empty strings should be allowed in my opinion.

Example of how to use this feature
In my use case some of the lists are embedded in a context where the title is legitimately empty and shouldn't be displayed. Currently it isn't possible to leave the title truly empty without overriding the template, which isn't very flexible. A dirty workaround is to set the title to a single space, which is obviously less than ideal. Setting the title to a false will produce an empty title, but this isn't an intended behavior given the function parameter type.

Scope
I propose to allow empty strings to be considered as valid titles, like in old versions. This makes the intent clear:

  • null = "I don't have an explicit title" vs.
  • '' = "The title is known to be an empty string".

This may constitute a small BC break unfortunately. I don't think introducing any more complexity for this is worth it, unless someone has a better idea. cc @Lustmored as the empty() behavior change looks unintentional as part of the PR.

WDYT?

@Lustmored
Copy link
Contributor

I think the problem comes from the fact that internally EA is currently using Translatable objects to store translatable content and therefore passing an empty string would produce a TranslatableMessage object with empty string, so that it's not and empty value anymore. It'd get weird during translation, but could work for sure.

Alternatively you can pass a TranslatableMessage object with an empty string as a value (Crud::setPageTitle($pageName, t(''))) and you will probably get what you need. But I'm not sure, you'd have to experiment with that.

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 a pull request may close this issue.

3 participants