46 bytes, text/x-github-pull-request
|Details | Review | Splinter Review|
122.37 KB, image/png
29.13 KB, image/png
60.07 KB, image/png
This bug is a simple CSS fix that impacts user experience in contacts a lot, and should make it into 2.1.
Created attachment 8483576 [details] [review] Github PR
triage: identifiable UI flaw. refer to below screenshot. https://bug1061945.bugzilla.mozilla.org/attachment.cgi?id=8483702
Sorry for the late point. Description of this bug was incomplete. We had some problems with the layout of the list, (shifting images to the right), but also with the top of the list. I'm changing the description for this. Sorry again for the late advice
Comment on attachment 8483576 [details] [review] Github PR Could we fix here the fact that the list it self is to close to the top (contacts search). In the case of favourites you could even see how the icon is cut in the top. For normal letters you'll see as well how the fixed header with the letter is as well pretty close to the search.
Can you include example STR to generate this bug?
Francisco, I updated the list separation between the search box and the list.
Created attachment 8486986 [details] ice contact.png Hi Sergi, I also have same problem for ICE contacts. The icon is pretty close to the search bar as well. I hope we can have some space between the icon and the search bar like the spec I attached. Is this something can be fixed at here? Thanks!
Created attachment 8487072 [details] Screenshot Hi Fang, It looks good to me, check the attachment I just added. Maybe Francisco can confirm about the space being correct. In case it is, do you give a green light to let this one in? Thanks, Sergi
(In reply to Sergi Mansilla [:sergi] (Telenor) from comment #11) > Created attachment 8487072 [details] > Screenshot > > Hi Fang, > > It looks good to me, check the attachment I just added. Maybe Francisco can > confirm about the space being correct. In case it is, do you give a green > light to let this one in? > > Thanks, > Sergi Wow! looks great!! Thanks!!
Comment on attachment 8483576 [details] [review] Github PR Hi Sergi, still can see the start image pretty close to the top: http://imgur.com/3OAFbpo Also, could you rebase the patch to have the ICE group in the top? Thanks!
Comment on attachment 8483576 [details] [review] Github PR Solved the favorites header space and rebased to latest master
Comment on attachment 8483576 [details] [review] Github PR Perfect, thanks a lot for the changes. Only thing before merging is that the linter job is red :( ... but cannot see anything why it should be tbh.
Merged at 9cfd2a4278a9a4f56c11c5de25a79f89862ad001
Please request Gaia v2.1 approval on this when you get a chance.
Comment on attachment 8483576 [details] [review] Github PR [Approval Request Comment] [Bug caused by] (feature/regressing bug #): Feature landed [User impact] if declined: Horrible user experience, completely broken in contact list with images [Testing completed]: Smoke test passed [Risk to taking this patch] (and alternatives if risky): Pretty low, just modifying 2 lines of css [String changes made]:
Comment on attachment 8483576 [details] [review] Github PR low risk css change fixing a 2.1 regression.
Tested and working 2.2 Flame User Gecko-5cf7098 Gaia-700f481 2.1 Flame User Gecko-5cf7098 Gaia-700f481
Created attachment 8525174 [details] verify_screenshot This issue can't repro on Flame 2.1 See attachment: verify_screenshot.png Reproducing rate: 0/5 Flame 2.1 versions: Gaia-Rev 1b231b87aad384842dfc79614b2a9ca68a4b4ff3 Gecko-Rev https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/95fbd7635152 Build-ID 20141118001204 Version 34.0 Device-Name flame FW-Release 4.4.2 FW-Incremental eng.cltbld.20141118.035447 FW-Date Tue Nov 18 03:54:58 EST 2014 Bootloader L1TC00011880