IBX-1674: Empty select option for dropdown #109
Conversation
| value: '{{ value }}', | ||
| label: '{{ label }}', | ||
| })|e('html_attr') }}" | ||
|
|
GrabowskiM
force-pushed
the
IBX-1674-empty-select
branch
from
09dcba9 to
f452baf
Compare
| {% set custom_form = custom_form is defined ? custom_form : true %} | ||
| {% set translation_domain = translation_domain|default(false) %} | ||
| {% set value = value is defined ? value : null %} | ||
| {% set multiple = multiple is defined ? multiple : false %} |
There was a problem hiding this comment.
nitpick question: why not just multiple|default(false)?
| {% set placeholder = placeholder|default(default_placeholder) %} | ||
| {% set label = placeholder %} | ||
|
|
||
| {{ _self.get_translated_label(placeholder, translation_domain )}} |
There was a problem hiding this comment.
Aren't you translating the placeholder twice? It looks like:
- you assign translated label to
default_placeholder - you pass it as
placeholdertoget_translated_labelwhere it is translated again whentranslation_domainis not false.
I guess that from the backend you are getting a placeholder that is not translated?
I'm also not sure whether we need get_translated_label as a separate macro for such a small thing.
There was a problem hiding this comment.
On one hand it's small thing, but it's used in five different places, its shorthand if, etc., I think it looks neater with macro
| <li class="ibexa-dropdown__selected-item ibexa-dropdown__selected-item--predefined ibexa-dropdown__selected-overflow-number" hidden></li> | ||
| </ul> | ||
| <div class="ibexa-dropdown__items"> | ||
| {% if choices_flat|length >= min_search_items %} |
There was a problem hiding this comment.
I wonder, wouldn't it be better to hide search with CSS class? Then you can always change decision in JS.
GrabowskiM
force-pushed
the
IBX-1674-empty-select
branch
from
a16e429 to
37397b3
Compare
GrabowskiM
force-pushed
the
IBX-1674-empty-select
branch
from
37397b3 to
e9fd9a6
Compare
| {% endmacro %} | ||
|
|
||
| {% macro get_translated_placeholder(placeholder, translation_domain) %} | ||
| {% if placeholder is defined %} |
There was a problem hiding this comment.
Can you pass a variable that is not defined (in this case placeholder) to a macro?
There was a problem hiding this comment.
You cannot, as discussed in private :)
GrabowskiM
force-pushed
the
IBX-1674-empty-select
branch
from
e9fd9a6 to
5f600cd
Compare
JIRA ref: https://issues.ibexa.co/browse/IBX-1674
Related PRs: