Closed Bug 1053119 Opened 5 years ago Closed 5 years ago

[Messages] Adding contact images in Thread list edit mode of SMS app and modification in existing Transition/Animations

Categories

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

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: rishav_, Assigned: rishav_, Mentored)

Details

Attachments

(6 files, 1 obsolete file)

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:31.0) Gecko/20100101 Firefox/31.0 (Beta/Release)
Build ID: 20140715214327

Steps to reproduce:

In SMS app there are contact images in thread list mode.
Now here we are adding the contact images in thread list edit mode too.

Refer comment 46 onwards of Bug 1020306 (https://bugzilla.mozilla.org/show_bug.cgi?id=1020306) to see the discussion.
Summary: Adding contact images in Thread list edit mode of SMS app → Adding contact images in Thread list edit mode of SMS app and modification in existing Transition/Animations
While reviewing the patch in bug 1020306, I've seen that when we display the edit mode, we change some properties that we shouldn't change. They trigger a reflow. Examples are: "left", "padding".

We need to change only "opacity" and "transform" to make the transition happen on the GPU, otherwise the animation is lost on the device.

Honestly, I can't find a way to make it work if we want to show the contact picture and keep the ellipsis (because showing the ellipsis makes it necessary to reduce the width of the container; and to reduce the width of the container we need a reflow).
Status: UNCONFIRMED → NEW
Ever confirmed: true
Mentor: gsvelto, felash
Hi jenny
As we here planning to experiment with edit mode. So i want know that as we are taliking about consistency accross app and for sms app we are taking email app as reference. Are you sure that email app won't change their interaction or mode. Because if we start working taking email as reference, and if later email app got change or did some modification then whole change in sms has no meaning.
So it will be great if you provide info on this.Also ni? for .. should we follow email app or juliew idea to implement edit mode using opacity and transform only?

Thanks
Flags: needinfo?(jelee)
Hi juliew
why i am not able to assign it to myself. Do i don't have any permissions?
Thanks
Hello Kumar, I don't feel strongly about following the email's transition. Since there's technical difficulty, feel free to follow Julien's instruction and proceed. Thanks!
Flags: needinfo?(jelee)
Hi jenny and julienw
After seeing email app, Following thing came in my mind which i would to share :

1 .Is there need of dialog box appears when we tap on option button (In thread list mode, there is option icon on top right corner).
2. as after tapping on option icon we get two option: select threads and setting.
3. Select thread icon can be kept in place of option icon and setting icon in top left most corner.
4. if in future more option needed to add then we will replace setting icon with Drop down option (something like in email app).

Here i am not saying to align the sms app with email app as features and number of features is different in email app.(Also we don't know about there plan of change in email app)

Basically my idea is removal of confirmation or dialog box. As we are going to implement undo action where we will remove existing confirmation dialog boxes. So it will be good to remove dialog boxes where it's possible as we have other alternative.

Hope this will help in refactoring the existing edit mode with opacity and transform and no longer need of 'reflow'.

Thanks
Flags: needinfo?(jelee)
Flags: needinfo?(felash)
Hi Julienw
In Thread list edit mode i refactored the css and only opacity & transform is used.But for including contact images in thread list edit mode , we have to use margin.I think it's ok if we use margin in one place because at other places are refactored using opacity and transform.
What do you say?
Thanks
Attached image Screen Shot 2014-08-07 at 22.28.01.png (obsolete) —
Here i attached screen shots after adding contact pictures.
Hi julienw
Here is patch .Have a look on this and give your feedback.

Here only transform and opacity is used. All other css stuff has been removed. Only at one place i used margin(because in contact image, it's not possible without using margin). 
I think adding contact image in edit mode will be good as it makes difference of only two letters (of sender name) compared to thread list mode. So it is not shortning the name much.
Thanks
Attachment #8474128 - Flags: feedback?(felash)
please assign the bug to me :)
Assignee: nobody → rishav006
Status: NEW → ASSIGNED
(In reply to kumar rishav from comment #5)
> Hi jenny and julienw
> After seeing email app, Following thing came in my mind which i would to
> share :
> 
> 1 .Is there need of dialog box appears when we tap on option button (In
> thread list mode, there is option icon on top right corner).
> 2. as after tapping on option icon we get two option: select threads and
> setting.
> 3. Select thread icon can be kept in place of option icon and setting icon
> in top left most corner.
> 4. if in future more option needed to add then we will replace setting icon
> with Drop down option (something like in email app).
> 
> Here i am not saying to align the sms app with email app as features and
> number of features is different in email app.(Also we don't know about there
> plan of change in email app)
> 
> Basically my idea is removal of confirmation or dialog box. As we are going
> to implement undo action where we will remove existing confirmation dialog
> boxes. So it will be good to remove dialog boxes where it's possible as we
> have other alternative.
> 
> Hope this will help in refactoring the existing edit mode with opacity and
> transform and no longer need of 'reflow'.
> 
> Thanks

Hi Kumar, can you provide a prototype so I can actually try out what you described above? It not clear to me how things would work. Thanks!
Flags: needinfo?(jelee)
Comment on attachment 8474128 [details] [review]
Bug 1053119 - Adding contact images in Thread list edit mode of SMS app and modification in existing Transition/Animations

I'm gonna redirect the feedback to Steve. Sorry that I didn't find the time to work on this :(

Steve if you're too busy please redirect to Oleg, but I think you're best suited here because you can ask things to Jenny more easily.

The goal is to eliminate reflows when displaying the edit mode, so that we see animations. I'm not sure this is easy to do with the menu, maybe we'd have to change how the menu is displayed? If you change how the menu is displayed, take care to accessibility as well (it should be hidden for screenreaders too).

Thanks !
Attachment #8474128 - Flags: feedback?(felash) → feedback?(schung)
Flags: needinfo?(felash)
Comment on attachment 8474128 [details] [review]
Bug 1053119 - Adding contact images in Thread list edit mode of SMS app and modification in existing Transition/Animations

Comments on the github. I think you will need 2 different margin-right for w/ photo and w/o photo cases if you use transform for list instead.
Attachment #8474128 - Flags: feedback?(schung)
Attachment #8474128 - Flags: feedback?(schung)
Hi steve
I have fixed the issue , you mentioned on github.Have a look.
Thanks
Attachment #8474125 - Attachment is obsolete: true
Comment on attachment 8474128 [details] [review]
Bug 1053119 - Adding contact images in Thread list edit mode of SMS app and modification in existing Transition/Animations

The patch works fine, thnaks! However I still think the text eclipse is still too close to the contact photo, so I'll let visual to judge the result.
Attachment #8474128 - Flags: feedback?(schung) → feedback+
Comment on attachment 8476984 [details]
Screen Shot 2014-08-06 at 03.12.22.png

Hi Fang, this patch will change some styling of the layout, but it should not have further differentiation than your latest visual spec. In this edit mode case, do you think the space between text eclipse and contact image is enough?
Attachment #8476984 - Flags: ui-review?(fshih)
Hi steve
I intentionally did that (text eclipse too close to contact photo). If i will try to give space, it will shorten the text by 2 to 3 letters. As a result name will get more shortened.
Thanks
Comment on attachment 8476984 [details]
Screen Shot 2014-08-06 at 03.12.22.png

Hi Steve, Overall it looks good. Can you leave 10px space between text eclipse and contact image? Also, I think dividers should be left aligned with contact name and shorter, The length should be ended in the same position with text. Thanks!
Attachment #8476984 - Flags: ui-review?(fshih) → ui-review-
Hi Fang
I didn't get this..Can you explain more?
> Also, I think dividers should be left aligned
> with contact name and shorter, The length should be ended in the same
> position with text. 

Thanks!
Flags: needinfo?(fshih)
Hi fang
I have attached the screen shot. Is this what you saying?
I will update the patch if it's okay :)
Thanks
Attached image bug_divider.png
Hi Kumar, Sorry for the confusion! I've attached an image to show the correct divider. Thanks!
Flags: needinfo?(fshih)
Hi Fang
If the divider should be shorter in thread list edit mode...Then shouldn't it be shorter for thread mode too?
To keep the divider uniform throught the different mode as in thread list mode divider is longer.
Thanks
Flags: needinfo?(fshih)
Attachment #8474128 - Flags: review?(schung)
(In reply to kumar rishav (:rishav_) from comment #23)
> Created attachment 8479722 [details]
> Screen Shot 2014-08-04 at 14.47.03.png
> 
> Hi Fang
> If the divider should be shorter in thread list edit mode...Then shouldn't
> it be shorter for thread mode too?
> To keep the divider uniform throught the different mode as in thread list
> mode divider is longer.
> Thanks

Hi Kumar, Yes, that is something I would like to adjust as well. The thread mode divider has shorter design in the current visual spec. But I guess we can only make adjustments in other thread mode refresh bug right? Beside that, the spacing between contact photo and text I think is good now!! Thanks for your help Kumar! : )
Flags: needinfo?(fshih)
Comment on attachment 8474128 [details] [review]
Bug 1053119 - Adding contact images in Thread list edit mode of SMS app and modification in existing Transition/Animations

r=me, thanks!
Attachment #8474128 - Flags: review?(schung) → review+
In master : db6932ad0c2aefbcccfb3f4127b3fcdc6818b1e9
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Summary: Adding contact images in Thread list edit mode of SMS app and modification in existing Transition/Animations → [Messages] Adding contact images in Thread list edit mode of SMS app and modification in existing Transition/Animations
You need to log in before you can comment on or make changes to this bug.