Closed Bug 1043318 Opened 8 years ago Closed 7 years ago

[Dialer] [Keypad] On suggestion item when the text goes outside the edge, you can drag the upper part and move the whole screen.

Categories

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

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
2.1 S1 (1aug)

People

(Reporter: paco, Assigned: paco)

References

(Blocks 1 open bug)

Details

Attachments

(3 files)

Attached file Issue on video
No description provided.
Assignee: nobody → pacorampas
Attached file patch in github
Attachment #8461459 - Flags: review?(anthony)
See Also: → 1030623
I'm not reproducing this issue. I have a very long contact and it has an ellipsis as expected. Could you give clearer STRs ?

Both si__name and si__details have an ellipsis class [1]. And that class adds an |overflow: hidden|. So I'm surprised to see this.

[1] https://github.com/mozilla-b2g/gaia/blob/fadfafa17f5175203b8b9457bfb95e5816f54f58/apps/communications/dialer/elements/suggestion-item.html#L4-L5
[2] https://github.com/mozilla-b2g/gaia/blob/fadfafa17f5175203b8b9457bfb95e5816f54f58/apps/communications/dialer/style/suggestion.css#L92
1- I have saved a contact with a long name.
2- I have searched the contact on keypad

Result: Appear a suggestion with horizontal scroll.

I have installed a flame build of yesterday. With old builds doesn't happen.
And I have gaia master.
(In reply to Anthony Ricaud (:rik) from comment #3)
> Both si__name and si__details have an ellipsis class [1]. And that class
> adds an |overflow: hidden|. So I'm surprised to see this.


When using flexbox and text-overflow: ellipsis, you need overflow: hidden in the parent of the text you want to truncate.
I have doublechecked that with Arnau.
BTW: The suggestion layer is outside of phone-number-view-container. I think we can improve the code if we include the suggestion item inside it. We can do it here or open a new bug. What do you think?
OK, I wasn't seeing because this is a regression of a Gecko change.
OS: Mac OS X → All
Hardware: x86 → All
Target Milestone: --- → 2.1 S1 (1aug)
Daniel: Can you help us verify if it's a Gecko or Gaia bug? I tried reading the spec but it's a collection of links to definitions and couldn't be sure.

We have this:
<div style="flex: 1">
  <div style="overflow: hidden">Very[…]long</div>
  <div style="overflow: hidden">not long</div>
</div>

Because all children are |overflow: hidden|, do we really have to set overflow: hidden or min-width: 0 on the container?
Flags: needinfo?(dholbert)
(In reply to Anthony Ricaud (:rik) from comment #8)
> Because all children are |overflow: hidden|, do we really have to set
> overflow: hidden or min-width: 0 on the container?

Is the <div style="flex: 1"> overflow:hidden?

That's div is the one whose "overflow" property matters for this min-sizing behavior, because that's the flex item. (I'm assuming its a flex item [child-of-a-flex-container], because it has flex:1 set on it, which only does anything on flex items.)

Its parent (the flex container) will ask for its min-content width, and it returns the min-content width of its widest child, which is the length of Very[...]long).  That ends up being <div style="flex: 1">'s min-width, so it overflows its flex-container-parent.

It sounds to me like the <div style="flex: 1"> wants "min-width: 0".
Flags: needinfo?(dholbert)
Comment on attachment 8461459 [details] [review]
patch in github

Thanks Daniel. r+ for setting overflow: hidden then.
Attachment #8461459 - Flags: review?(anthony) → review+
I think just using "min-width:0" really might be better.  "overflow" is a slightly more heavyweight solution (requires additional frames in the frame tree, to support scrolling), and it's only useful if you're expecting the flex item's contents to overflow outside the flex item's final size *and* you have a way you want to handle that overflow (scrolling, ellipsis, etc).  Neither of those seem to be the case here -- instead, the children themselves are sized to be no wider than the flex item (I think?) and they handle their own overflow with overflow:hidden. So I'm not sure it's useful to add another higher-level layer of "overflow:hidden" here.

Maybe I'm missing something, though.
Merged: ba3654571c09e539a0c8584f29f53438563bb920
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Paco, can you start putting demos of work like this here?
https://wiki.mozilla.org/FirefoxOS/Comms/Dialer/Sprint/v2.1-S1#Demos
Flags: needinfo?(pacorampas)
rik or paco, could you look at comment 11, and possibly update the CSS accordingly?

I really think adding "overflow:hidden" on a flex item is not the best workaround for this bug, except in cases where we're explicitly handling overflow *on the flex item*. (and that's not true of this case) It should be more lightweight and performant to use "min-width:0".
Flags: needinfo?(anthony)
Ok Doug, I'm going to put there my demos.
Flags: needinfo?(pacorampas)
(In reply to Paco Rampas [:paco] from comment #15)
> Ok Doug, I'm going to put there my demos.

Thanks!

(In reply to Daniel Holbert [:dholbert] from comment #14)
> rik or paco, could you look at comment 11, and possibly update the CSS
> accordingly?
> 
> I really think adding "overflow:hidden" on a flex item is not the best
> workaround for this bug, except in cases where we're explicitly handling
> overflow *on the flex item*. (and that's not true of this case) It should be
> more lightweight and performant to use "min-width:0".

Do you think we should back this out, or do a followup?
Daniel, very sorry, I have read the comment 11 after merge. I can improve the patch as you said if Antony is agree.
(In reply to Doug Sherk (:drs) from comment #16)
> Do you think we should back this out, or do a followup?

Followup is fine, I think. overflow:hidden should still work, & isn't *too* bad; it's just that min-width:0 should be better for this situation.

(In reply to Paco Rampas [:paco] from comment #17)
> Daniel, very sorry, I have read the comment 11 after merge. I can improve
> the patch as you said if Antony is agree.

Thanks! Anthony, does this sound OK to you?
Yeah, let's open a follow up bug to deal with this. Can you take care of it Paco? And ask review to Doug.
Flags: needinfo?(anthony)
Depends on: 1047224
You need to log in before you can comment on or make changes to this bug.