1099397 - Add an empty banner to contain the favorite contacts list

Status: RESOLVED WONTFIX

(Firefox OS Graveyard :: Gaia::Dialer, defect)

Description

[Doug Sherk (:drs) (inactive)]

The objective of this bug is to add a very simple banner to the top of the call log, which will eventually contain the favorite contacts. The reason this is a separate task is because there are many unrelated bugs which can be worked on but which depend on there being at least an empty container to work with.

Comment 1

[Paco Rampas [:paco]]

Attachment: patch in github

Hi Doug,

Here the patch, only two things to comment: 
- I can't found any spec with sizes and colors. I only have this pdf with WF:
https://bug1097757.bugzilla.mozilla.org/attachment.cgi?id=8525940

So, if there are specs, where can I find it? Now, I set it sizes invented for me.

- Also, I think that is better building the call log layout with flex because the favorite layer will be hide with scroll. With this patch we must change two values for hide it.
https://github.com/mozilla-b2g/gaia/pull/26445/files#diff-b8093e9e6aaf5257861ae497a6909b72R101
https://github.com/mozilla-b2g/gaia/pull/26445/files#diff-b8093e9e6aaf5257861ae497a6909b72R131

With flex we can build a layout adaptable in real time only changing the height of favorite container. What do you think?


Thanks.

Comment 2

[Paco Rampas [:paco]]

I forgot to check NI flag in comment 1

Comment 3

[Doug Sherk (:drs) (inactive)]

(In reply to Paco Rampas [:paco] from comment #1)
> Here the patch, only two things to comment: 
> - I can't found any spec with sizes and colors. I only have this pdf with WF:
> https://bug1097757.bugzilla.mozilla.org/attachment.cgi?id=8525940
> 
> So, if there are specs, where can I find it? Now, I set it sizes invented
> for me.

Yes, you're right. Unfortunately, we don't have any VD specs for this work yet.

Since so much work is dependent on just having this empty banner, I think it's best if we land this with values to your discretion and then tune it later once we have the spec.

I've filed bug 1104899 to track the followup work here.

> - Also, I think that is better building the call log layout with flex
> because the favorite layer will be hide with scroll. With this patch we must
> change two values for hide it.
> https://github.com/mozilla-b2g/gaia/pull/26445/files#diff-
> b8093e9e6aaf5257861ae497a6909b72R101
> https://github.com/mozilla-b2g/gaia/pull/26445/files#diff-
> b8093e9e6aaf5257861ae497a6909b72R131
> 
> With flex we can build a layout adaptable in real time only changing the
> height of favorite container. What do you think?

I wrote an outline for some followup work after this in bug 1099420 comment 0. I believe that the proposal in attachment 8528325 [details] [review] will actually work fine with our future work, as we can translate the entire call log+favorite contacts container all as one to show and hide the favorite contacts banner. So what I'm getting at is that, to perform this animation, we don't need to alter the heights as you suggested. Can you think of any other reason to do this with flexbox?

Comment 4

[Paco Rampas [:paco]]

> I think it's best if we land this with values to your discretion and then tune it later once we have the > spec.
So, I'm going to put the review flag for landing after getting the R+  ;)
Thanks you.

> So what I'm getting at is that, to perform this animation, we don't need to alter the heights as you 
> suggested. Can you think of any other reason to do this with flexbox?
Yes, the performance will be better with the solution that you comment. I don't know what I thought. Sorry.

Comment 5

[Paco Rampas [:paco]]

Comment on attachment 8528325 [details] [review]
patch in github

See the previous comment 4.
Thanks

Comment 6

[Doug Sherk (:drs) (inactive)]

Comment on attachment 8528325 [details] [review]
patch in github

Looks good, thanks! Trying it gave me the idea to add static text saying "this blank space added intentionally in bug 1099397". Could you please do that before landing?

As always, please record a demo and post it on the sprint demo page:
https://wiki.mozilla.org/FirefoxOS/Comms/Dialer/Sprint/v2.2-S1#Demos

Comment 7

[Paco Rampas [:paco]]

mregerd: 7cc460af0f6f491d1afa6b6043cdea5025f0e15f

Comment 8

[Paco Rampas [:paco]]

He Doug,
After merge I have seen the color of the text: "this blank space added intentionally in bug 1099397" is white and the background is a grey, so it is difficult to read because there isn't enough contrast. I don't know if that is really important, but is my fault because I didn't test the patch before update it. So, I apologise for that.

I have marked the bug as fixed, but if it is necessary reopen it let me know. 

Thanks and sorry.

Comment 9

[Doug Sherk (:drs) (inactive)]

It's ok, don't worry about it. We can sneak a color change into bug 1099366.

Comment 10

[Kevin Grandon :kgrandon]

Hey Guys - what is the bug number I should follow for implementing this feature? Having this on master and in my phone that I use everyday is a bit awkward.

Ideally unfinished features would be landed behind a preference or setting in the future.

Comment 11

[Doug Sherk (:drs) (inactive)]

Please back this out. We'll land it under a pref at a later time.

Comment 12

[Carsten Book [:Tomcat]]

(In reply to Doug Sherk (:drs) (use needinfo?) from comment #11)
> Please back this out. We'll land it under a pref at a later time.

just to confirm you want d4efc296bcf392ebfab24952a453015cae3a68c0 to get out right ?

Comment 13

[Ryan VanderMeulen [:RyanVM]]

Reverted.

Master: https://github.com/mozilla-b2g/gaia/commit/1c636b94778d83cabccfa877400941ab4ef776f5

Comment 14

[BMO Automation]

Firefox OS is not being worked on