Closed Bug 871454 Opened 7 years ago Closed 7 years ago

[Email] Long email address truncated in action menu header when contact bubble is clicked

Categories

(Firefox OS Graveyard :: Gaia::E-Mail, defect, P3)

ARM
Gonk (Firefox OS)

Tracking

(blocking-b2g:leo+, b2g18+ fixed)

RESOLVED FIXED
1.1 QE2 (6jun)
blocking-b2g leo+
Tracking Status
b2g18 + fixed

People

(Reporter: leo.bugzilla.gaia, Assigned: mihai)

References

()

Details

(Whiteboard: [TD-27417], c= , MiniWW)

Attachments

(2 files, 1 obsolete file)

1. Title : Long email id not shown completely in contact selection screen.
2. Precondition : Email should be working 
3. Tester's Action:  Launch Email -> Open an email -> Type long email id-> Select on the bubble
4. Detailed Symptom (ENG.) : email id is not displayed properly on contact selection screen
5. Expected :Email id should be displayed properly.
6.Reproducibility: Y
           1)Frequency Rate : 100%
7.Gaia Master/v1-train : Reproduced
8.Gaia Revision: 393b3f57822ae0f34055c6a6060f1433136bafa0
9.Personal email id:  psingapati@gmail.com
Long email id is not shown completely
Target Milestone: --- → 1.1 QE2
Summary: [Email] Long email id not shown completely in contact selection screen. → [Email] Long email address truncated in action menu header when contact bubble is clicked
Any suggestion on how the email string should be displayed?

I'm thinking the solution can be showing the email either truncated with ellipsis (i.e. "thisisalongemailaddress@y...") which would fit well with the title bar height, or on multiple lines, which might still not fit in the title bar height.
Assignee: nobody → mihai
Flags: needinfo?(jcarpenter)
Priority: -- → P3
Attached file Pull Request pointer (obsolete) —
This issue is fixed by providing form dialog header overflow: auto; 
In composer screen , we are showing ellipsis to long email id bubble to fit that properly in the composer screen width as part of the issue 871449 fix.
So, we can show the complete email id in the form header.

Please review it.
Attachment #749250 - Flags: review?(bugmail)
Hi Mihai,
I have uploaded patch to this issue.
Please un assign yourself, I will take it.

Thanks
Flags: needinfo?(mihai)
Hi psingapati, I highly appreciate you working on these bugs, however it is best if you ask before starting to work on a patch, since other people might have also started to work on it -- which is me in this case.
Assignee: mihai → psingapati
Flags: needinfo?(mihai)
Assignee: psingapati → leo.bugzilla.gaia
Attachment #749250 - Attachment mime type: text/plain → text/html
Comment on attachment 749250 [details]
Pull Request pointer

Adding scrolling seems like a very good idea for fixing this since it makes the information accessible, but it's a UX call.
Attachment #749250 - Flags: review?(rmacdonald)
Attachment #749250 - Flags: review?(bugmail)
Attachment #749250 - Flags: feedback+
blocking-b2g: --- → -
tracking-b2g18: --- → +
Sorry - I'm having troubles downloading patches right now (phone won't reboot) and I can't replicate this in aurora because it doesn't let me set up an account. 

Mihai, I know you had to do this with music, but would it be possible to record and post a video?
Flags: needinfo?(jcarpenter)
(In reply to Rob MacDonald [:robmac] from comment #7)
> Sorry - I'm having troubles downloading patches right now (phone won't
> reboot) and I can't replicate this in aurora because it doesn't let me set
> up an account. 
> 
> Mihai, I know you had to do this with music, but would it be possible to
> record and post a video?

I think the screenshot should probably provide you with enough context here.  When you click on a name/mail address bubble in either the message reader or the composer, we show an action menu.  The e-mail address is displayed at the top of the action menu.

What do we do if the e-mail is wider than the screen?
1) ellipsis it
2) wrap it
3) add horizontal scrolling to the e-mail address

This patch currently proposes option 3.  This makes the information available (as compared to option 1, which hides information) and without potentially messing up the rest of the display (which option 2 could result in), but is not particularly discoverable.


Also, how are you running things on aurora?  Currently, the easiest way to run the e-mail app in Firefox is to do "make DEBUG=1 NOFTU=1 profile", then run firefox against that profile by doing "firefox -profile /path/to/profile" which uses the fancy extension stuff kgrandon and friends created.
Flags: needinfo?(rmacdonald)
Thanks Andrew. I've been having ongoing problems applying patches and it's probably best if I figure this out with Casey when he's back in the office next week. It seems like, no matter what the patch, the phone dies before reaching the Firefox OS splash screen. :(

I agree with option 3, however, when we used the marquee in Music previously, it was choppier than we were willing to accept in that context. Although I haven't seen it, I presume it would be the same here unless, Mihai, you've found a miracle cure. :)

Even if the animation is slightly choppy in this case I'm inclined to say go for it because (A) this screen is deeper in the UI than the marquee used in Music and (B) the alternatives are not great. Mihai, I'd propose using the same animation and speed that you used for Music with this bug. The initial view shows the string aligned left, after a couple of seconds it begins to marquee to the left.

Hopefully I can get these patch issues resolved early next week. Until then flag me if you have any additional questions or concerns.
Flags: needinfo?(rmacdonald)
Whiteboard: [TD-27417] → [TD-27417], c=
Hi Rob, excuse my delayed response,

(In reply to Rob MacDonald [:robmac] from comment #9)
> I agree with option 3, however, when we used the marquee in Music
> previously, it was choppier than we were willing to accept in that context.
> Although I haven't seen it, I presume it would be the same here unless,
> Mihai, you've found a miracle cure. :)

The current patch, proposed by Leo, is simply adding the horizontal scroll functionality for the header, that is a scrollbar in the lower part of the header and not the slow right-to-left marquee that I was working on for the Music app :)

By testing this parch, the solution opted for (i.e. adding horizontal scrolling) doesn't seem to help in terms of notifying the user of a truncated email address, since the horizontal scrollbar is hidden and the action menu looks exactly as in the screenshot from comment 1. Only if the user tries to scroll the header horizontally, he can see the whole email address.

> Even if the animation is slightly choppy in this case I'm inclined to say go
> for it because (A) this screen is deeper in the UI than the marquee used in
> Music and (B) the alternatives are not great. Mihai, I'd propose using the
> same animation and speed that you used for Music with this bug. The initial
> view shows the string aligned left, after a couple of seconds it begins to
> marquee to the left.

I would be more than happy to provide a patch with the marquee, and I can even bet it will be smooth since there aren't any CPU/GPU intensive tasks happening in the action menu, as opposed to the Music Player.

Let me know what you think. Thanks!
Flags: needinfo?(rmacdonald)
Thanks, Mihai... If we can do the patch with the marquee, similar to what you did for music, then I think that would work quite well. - Rob
Flags: needinfo?(rmacdonald)
Comment on attachment 749250 [details]
Pull Request pointer

Proposing use of marquee as per comment 11. Thanks!
Attachment #749250 - Flags: review?(rmacdonald) → review-
Hi psingapati, should I take over this and add the marquee I worked on for the Music app (see bug 868549), or do you want to update your patch?
Flags: needinfo?(psingapati)
Hi Mihai,

Please take it. 

Thanks.
Flags: needinfo?(psingapati)
Whiteboard: [TD-27417], c= → [TD-27417], c= , MiniWW
As discussed in San Diego work week marking this as leo+.
blocking-b2g: - → leo+
Assignee: leo.bugzilla.gaia → mihai
Status: NEW → ASSIGNED
Include the marquee in the header for compose and message cards whose email address overflows the width of the header.

The modular implementation (marquee.js and marquee.css) allows the easy use of the marquee in any header of any app (if this will be the case for the future this should be moved from apps/email/[js|style] to shared/[js|style]).

A demo on how it looks on an unagi device is available at http://youtu.be/pr7TxJJkqZM.
Attachment #755310 - Flags: review?(bugmail)
Attachment #755310 - Flags: feedback?(rmacdonald)
See Also: → 868549
Attachment #749250 - Attachment is obsolete: true
Comment on attachment 755310 [details]
Pull Request #10073 - Add marquee for long emails in action menu header

Thanks for the patch!

There appears to be a low probability bug (under mozilla-b2g18) where the initial marquee-start animation won't actually be visible.  So if we have "foo@bar.com", rather than animating from its initial display position, it will stay there for (I believe) the duration of the first animation pass, then jump all the way to the right side of the screen and start working like normal from then on out.

I expect this has something to do with when the reflow actually occurs and the animation only having a 100% keyframe with the marquee-start case not having an explicit initial transform value.

As a potential life-cycle issue for the animation, because there is no cleanup logic, the DOM nodes will be held onto as a quasi-leak until the next animation pass.  Since the memory use is bounded and they are trivial nodes, it doesn't really matter, but it's worth noting.

If you want to try and address that, that would be great.  But this is already a clear improvement and can land as-is.
Attachment #755310 - Flags: review?(bugmail) → review+
(In reply to Andrew Sutherland (:asuth) from comment #17)
> Comment on attachment 755310 [details]
> Pull Request #10073 - Add marquee for long emails in action menu header
> 
> Thanks for the patch!
> 
> There appears to be a low probability bug (under mozilla-b2g18) where the
> initial marquee-start animation won't actually be visible.  So if we have
> "foo@bar.com", rather than animating from its initial display position, it
> will stay there for (I believe) the duration of the first animation pass,
> then jump all the way to the right side of the screen and start working like
> normal from then on out.
> 
> I expect this has something to do with when the reflow actually occurs and
> the animation only having a 100% keyframe with the marquee-start case not
> having an explicit initial transform value.
> 
> As a potential life-cycle issue for the animation, because there is no
> cleanup logic, the DOM nodes will be held onto as a quasi-leak until the
> next animation pass.  Since the memory use is bounded and they are trivial
> nodes, it doesn't really matter, but it's worth noting.
> 
> If you want to try and address that, that would be great.  But this is
> already a clear improvement and can land as-is.

Thanks for checking the patch, Andrew, I added an explicit "transform: translateX(0)" to .marquee-start to tackle the potential issue you highlighted, however it might be something more subtle than this since I couldn't reproduce it.

Will be waiting for Rob's feedback before merging to master. Thanks!
I'm going to needinfo Rob so he knows that additional feedback is still needed here, just to make this findable.
Flags: needinfo?(rmacdonald)
Comment on attachment 755310 [details]
Pull Request #10073 - Add marquee for long emails in action menu header

First, thanks for putting together the video - very much appreciated. In terms of my feedback, I had a chance to discuss with Mihai over IRC and came up with this...

1) Display the email address (as per video)
2) After ~3 seconds, start the marquee, moving from right to left (as per video)
3) NEW - Scroll until the end of the email address is displayed on the right margin of the title bar
4) NEW - Stop for three seconds... then...
5) NEW - Scroll in the opposite direction at the same speed.
6) NEW - Scroll until the the start of the email address is displayed on the left margin of the title bar (the original state)
7) Return to Step 2 and start all over again

Hopefully this makes sense. Flag me or ping me on IRC if you have any questions.

- Rob
Attachment #755310 - Flags: feedback?(rmacdonald) → feedback-
Flags: needinfo?(rmacdonald)
Mihai, if you can't fix this this week, please land the existing fix and file a follow-up bug to improve the UX.
(In reply to Andreas Gal :gal from comment #21)
> Mihai, if you can't fix this this week, please land the existing fix and
> file a follow-up bug to improve the UX.

Hi Andreas, I'm working on this today so by tomorrow the latest it will be on master.
Comment on attachment 755310 [details]
Pull Request #10073 - Add marquee for long emails in action menu header

Updated the patch with an extension to the marquee that allows specifying the desired animation behavior ('scroll' or 'alternate', the latter following Rob's suggestion in comment 20) and also the animation timing function ('linear' for continuous constant speed animation -- as it was before, or 'ease' for slow start of the animation).

Set the animation behavior for message cards to 'alternate' and added the 'linear' timing function to compose-cards (email bubble tapped from compose screen) and the 'ease' timing function to message-cards (email bubble tapped from regular view-email screen) for proof-of-concept. The way these two types of animations look like on an unagi device can be consulted at http://youtu.be/WmVElGXvOQw.

Rob, let me know which timing function is desirable ('linear' or 'ease') and if everything looks good with the alternate animation behavior? Thanks!
Attachment #755310 - Flags: review?(bugmail)
Attachment #755310 - Flags: review+
Attachment #755310 - Flags: feedback?(rmacdonald)
Attachment #755310 - Flags: feedback-
Attachment #755310 - Flags: review?(bugmail) → review+
Comment on attachment 755310 [details]
Pull Request #10073 - Add marquee for long emails in action menu header

Big plus for using the "ease" approach shown in the first example the video. It has a great feel to it. Thanks so much for your attention to detail on this one.
Attachment #755310 - Flags: feedback?(rmacdonald) → feedback+
Landed on master:
https://github.com/mozilla-b2g/gaia/commit/c636623e3ed9e6e55dc58fea89491956b778aae5
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Uplifted c636623e3ed9e6e55dc58fea89491956b778aae5 to:
v1-train: 7cafcbaf45d874bb1cdda5887fce9b199fcf90ed
Flags: in-moztrap?
Flags: in-moztrap? → in-moztrap+
You need to log in before you can comment on or make changes to this bug.