Closed Bug 1099397 Opened 5 years ago Closed 2 years ago

Add an empty banner to contain the favorite contacts list

Categories

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

defect
Not set

Tracking

(b2g-v2.2 affected)

RESOLVED WONTFIX
Tracking Status
b2g-v2.2 --- affected

People

(Reporter: drs, Unassigned)

References

Details

(Whiteboard: [planned-sprint c=1])

Attachments

(1 file)

46 bytes, text/x-github-pull-request
drs
: review+
Details | Review
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.
Blocks: 1099420
Blocks: 1100559
Blocks: 1100561
Blocks: 1101302
Blocks: 1101314
Assignee: nobody → gtorodelvalle
Whiteboard: [planned-sprint c=?] → [planned-sprint c=1]
Assignee: gtorodelvalle → pacorampas
Attached file 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.
I forgot to check NI flag in comment 1
Flags: needinfo?(drs.bugzilla)
(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?
Blocks: 1104899
Flags: needinfo?(drs.bugzilla)
> 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 on attachment 8528325 [details] [review]
patch in github

See the previous comment 4.
Thanks
Attachment #8528325 - Flags: review?(drs.bugzilla)
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
Flags: needinfo?(pacorampas)
Attachment #8528325 - Flags: review?(drs.bugzilla) → review+
mregerd: 7cc460af0f6f491d1afa6b6043cdea5025f0e15f
Flags: needinfo?(pacorampas)
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.
Status: NEW → RESOLVED
Closed: 5 years ago
Flags: needinfo?(drs.bugzilla)
Resolution: --- → FIXED
It's ok, don't worry about it. We can sneak a color change into bug 1099366.
Flags: needinfo?(drs.bugzilla)
QA Contact: jlorenzo
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.
Flags: needinfo?(pacorampas)
Flags: needinfo?(drs.bugzilla)
Please back this out. We'll land it under a pref at a later time.
Flags: needinfo?(pacorampas)
Flags: needinfo?(drs.bugzilla)
Keywords: checkin-needed
(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 ?
Flags: needinfo?(drs.bugzilla)
Reverted.

Master: https://github.com/mozilla-b2g/gaia/commit/1c636b94778d83cabccfa877400941ab4ef776f5
Status: RESOLVED → REOPENED
Flags: needinfo?(drs.bugzilla)
Keywords: checkin-needed
Resolution: FIXED → ---
Target Milestone: 2.2 S1 (5dec) → ---
Assignee: pacorampas → nobody
Firefox OS is not being worked on
Status: REOPENED → RESOLVED
Closed: 5 years ago2 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.