Hide last updated data until date is populated.

Hide last updated data until date is populated.

This fixes an issue where "last updated" data would be displayed before date
was populated, showing some weird data. I tried to use "template if" to
restrict when this would be painted, but couldn't get that to work, so I
resorted to using styling instead.

NOTRY=true
BUG=402661
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=180100

[ojan, 2014-08-12 17:39:06]

https://codereview.chromium.org/466783002/diff/1/Tools/GardeningServer/ui/ct-...
File Tools/GardeningServer/ui/ct-last-updated.html (right):

https://codereview.chromium.org/466783002/diff/1/Tools/GardeningServer/ui/ct-...
Tools/GardeningServer/ui/ct-last-updated.html:14: <span class="{{ { hidden:
!visible } | tokenList }}">Updated {{ date | _deltaMinutes }} min ago @ {{ date
| _hours }}:{{ date | _minutes }}</span>
I think your <template if> problem is likely that you put the if on the outer
template. You can only put template directives on nested templates. I just poked
the polymer team to make this more clear in their documentation.

https://codereview.chromium.org/466783002/diff/1/Tools/GardeningServer/ui/tes...
File Tools/GardeningServer/ui/test/ct-last-updated-tests.html (right):

https://codereview.chromium.org/466783002/diff/1/Tools/GardeningServer/ui/tes...
Tools/GardeningServer/ui/test/ct-last-updated-tests.html:19: builder =
document.createElement('ct-last-updated');
s/builder/lastUpdated

https://codereview.chromium.org/466783002/diff/1/Tools/GardeningServer/ui/tes...
Tools/GardeningServer/ui/test/ct-last-updated-tests.html:34: builder =
document.createElement('ct-last-updated');
s/builder/lastUpdated

[teravest, 2014-08-12 18:12:45]

On Tue, Aug 12, 2014 at 11:39 AM,  <ojan@chromium.org> wrote:
>
>
https://codereview.chromium.org/466783002/diff/1/Tools/GardeningServer/ui/ct-...
> File Tools/GardeningServer/ui/ct-last-updated.html (right):
>
>
https://codereview.chromium.org/466783002/diff/1/Tools/GardeningServer/ui/ct-...
> Tools/GardeningServer/ui/ct-last-updated.html:14: <span class="{{ {
>
> hidden: !visible } | tokenList }}">Updated {{ date | _deltaMinutes }}
> min ago @ {{ date | _hours }}:{{ date | _minutes }}</span>
> I think your <template if> problem is likely that you put the if on the
> outer template. You can only put template directives on nested
> templates. I just poked the polymer team to make this more clear in
> their documentation.

Thanks! That's the problem I was having. Using an inner template, the
generated output looks weird-- the text is outside the template tags,
which is not what I would have expected...

<ct-last-updated date="{{ lastUpdateDate }}"></ct-last-updated>
  #shadow-root
    <template if="{{ visible }}"></template>
      #document-fragment
    </template>
    Updated 1 minute ago at 12:10
</ct-last-updated>

... but I'll figure out how to write a test to match that string.

>
>
https://codereview.chromium.org/466783002/diff/1/Tools/GardeningServer/ui/tes...
> File Tools/GardeningServer/ui/test/ct-last-updated-tests.html (right):
>
>
https://codereview.chromium.org/466783002/diff/1/Tools/GardeningServer/ui/tes...
> Tools/GardeningServer/ui/test/ct-last-updated-tests.html:19: builder =
> document.createElement('ct-last-updated');
> s/builder/lastUpdated
>
>
https://codereview.chromium.org/466783002/diff/1/Tools/GardeningServer/ui/tes...
> Tools/GardeningServer/ui/test/ct-last-updated-tests.html:34: builder =
> document.createElement('ct-last-updated');
> s/builder/lastUpdated
>
> https://codereview.chromium.org/466783002/

To unsubscribe from this group and stop receiving emails from it, send an email
to blink-reviews+unsubscribe@chromium.org.

[teravest, 2014-08-12 19:31:41]

https://codereview.chromium.org/466783002/diff/1/Tools/GardeningServer/ui/ct-...
File Tools/GardeningServer/ui/ct-last-updated.html (right):

https://codereview.chromium.org/466783002/diff/1/Tools/GardeningServer/ui/ct-...
Tools/GardeningServer/ui/ct-last-updated.html:14: <span class="{{ { hidden:
!visible } | tokenList }}">Updated {{ date | _deltaMinutes }} min ago @ {{ date
| _hours }}:{{ date | _minutes }}</span>
On 2014/08/12 17:39:05, ojan-only-code-yellow-reviews wrote:
> I think your <template if> problem is likely that you put the if on the outer
> template. You can only put template directives on nested templates. I just
poked
> the polymer team to make this more clear in their documentation.

I've changed this to use an inner template tag as well.

https://codereview.chromium.org/466783002/diff/1/Tools/GardeningServer/ui/tes...
File Tools/GardeningServer/ui/test/ct-last-updated-tests.html (right):

https://codereview.chromium.org/466783002/diff/1/Tools/GardeningServer/ui/tes...
Tools/GardeningServer/ui/test/ct-last-updated-tests.html:19: builder =
document.createElement('ct-last-updated');
On 2014/08/12 17:39:05, ojan-only-code-yellow-reviews wrote:
> s/builder/lastUpdated

Done.

https://codereview.chromium.org/466783002/diff/1/Tools/GardeningServer/ui/tes...
Tools/GardeningServer/ui/test/ct-last-updated-tests.html:34: builder =
document.createElement('ct-last-updated');
On 2014/08/12 17:39:05, ojan-only-code-yellow-reviews wrote:
> s/builder/lastUpdated

Done.

[ojan, 2014-08-12 21:27:06]

lgtm

https://codereview.chromium.org/466783002/diff/40001/Tools/GardeningServer/ui...
File Tools/GardeningServer/ui/ct-last-updated.html (right):

https://codereview.chromium.org/466783002/diff/40001/Tools/GardeningServer/ui...
Tools/GardeningServer/ui/ct-last-updated.html:9: <template if="{{ visible }}">
How about instead of adding a separate property, you make the default value of
date be null instead of new Date(0)? That way you can just check if="{{ date }}"
here and if the date is removed, then we stop displaying it again. Also you then
wouldn't need the dateChanged handler.

https://codereview.chromium.org/466783002/diff/40001/Tools/GardeningServer/ui...
Tools/GardeningServer/ui/ct-last-updated.html:30: var s = date.getMinutes() +
"";
Nit: date.getMinutes().toString() is more idiomatic JS I think.

[teravest, 2014-08-12 21:30:04]

https://codereview.chromium.org/466783002/diff/40001/Tools/GardeningServer/ui...
File Tools/GardeningServer/ui/ct-last-updated.html (right):

https://codereview.chromium.org/466783002/diff/40001/Tools/GardeningServer/ui...
Tools/GardeningServer/ui/ct-last-updated.html:9: <template if="{{ visible }}">
On 2014/08/12 21:27:05, ojan-only-code-yellow-reviews wrote:
> How about instead of adding a separate property, you make the default value of
> date be null instead of new Date(0)? That way you can just check if="{{ date
}}"
> here and if the date is removed, then we stop displaying it again. Also you
then
> wouldn't need the dateChanged handler.

Much better, thanks.

https://codereview.chromium.org/466783002/diff/40001/Tools/GardeningServer/ui...
Tools/GardeningServer/ui/ct-last-updated.html:30: var s = date.getMinutes() +
"";
On 2014/08/12 21:27:05, ojan-only-code-yellow-reviews wrote:
> Nit: date.getMinutes().toString() is more idiomatic JS I think.

Done.

[teravest, 2014-08-12 21:30:07]

The CQ bit was checked by teravest@chromium.org

[commit-bot: I haz the power, 2014-08-12 21:31:39]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/teravest@chromium.org/466783002/60001

[commit-bot: I haz the power, 2014-08-12 21:33:53]

Change committed as 180100