-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
Don't handle '0' as empty value in label columns in datatables #9996
Conversation
@@ -47,7 +47,7 @@ | |||
{% if row.getMetadata('html_label_prefix') %}<span class='label-prefix'>{{ row.getMetadata('html_label_prefix') | raw }} </span>{% endif -%} | |||
{%- if row.getMetadata('html_label_suffix') %}<span class='label-suffix'>{{ row.getMetadata('html_label_suffix') | raw }}</span>{% endif -%} | |||
{% endif %}<span class="value"> | |||
{%- if row.getColumn(column) %}{% if column=='label' %}{{- row.getColumn(column)|rawSafeDecoded -}}{% else %}{{- row.getColumn(column)|number(2,0)|raw -}}{% endif %} | |||
{%- if row.getColumn(column) or (column=='label' and row.getColumn(column) == "0") %}{% if column=='label' %}{{- row.getColumn(column)|rawSafeDecoded -}}{% else %}{{- row.getColumn(column)|number(2,0)|raw -}}{% endif %} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The UI tests are still running but just wondering in general should this be maybe something like is "0"
? (if that even works, Twig might throw an error)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You mean row.getColumn(column) is "0"
Tried that. Throws a Error: Unexpected token "string" of value "0" ("name" expected) in "@CoreHome/_dataTableCell.twig" at line 50
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if maybe sameas('0')
works maybe?
Otherwise isn't it basically the same as row.getColumn(column) or column=='label'
then? Also within this if there is already a {% if column=='label' %}{{- row.getColumn(column)|rawSafeDecoded -}}
maybe it could be splitted into two if
statements like
if column == label .... html. elseif row.getColumn ...html
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No it's not the same as the label column might still contain an empty string or null
- which I guess should still be presented as -
And row.getColumn(column) is same as("0")
works. But I'm not sure if that is quite better than comparing directly using ==
. Shouldn't we then also do ``column=='label'as
column is same as("label")` ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No it's not the same as the label column might still contain an empty string or null - which I guess should still be presented as -
I'm not sure how twig compares. When using two equal signs isn't ("0" == null) === true
? So currently row.getColumn(column) or (column=='label' and row.getColumn(column) == "0")
should be same as row.getColumn(column) or column=='label'
but not 100% sure. I don't think same as
is needed for label
. If a label can be an empty string or null
, then we should compare via is sameas('0')
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah ok. Now I understand what you wanted to point to.. I'll change it to is same as("0")
👍 |
fixes #9628