1089568 - [camera] outline offset to play button background

Status: RESOLVED FIXED

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

Comment 1

[Pavel Ivanov [:ivanovpavel][:pivanov] UX]

Attachment: patch for Gaia/master

Comment 2

[Punam Dahiya [:pdahiya]]

Thanks Pavel, that was quick! Passing the review to camera peer so that they are in loop. Also setting ui-review flag for Amy.

Justin, This is follow up bug of a similar fix done in gallery app video player in bug 994681. Please review.

Comment 3

[Amy Lee [:amylee] UX]

Comment on attachment 8511896 [details] [review]
patch for Gaia/master

Hi, 

The background grey circle should be inside the white outline circle. Right now it looks like it's larger than the white outline.

Comment 4

[Pavel Ivanov [:ivanovpavel][:pivanov] UX]

Comment on attachment 8511896 [details] [review]
patch for Gaia/master

Hey Amy, 
can you check the patch again?

Comment 5

[Amy Lee [:amylee] UX]

Attachment: 2014-10-28-15-41-39.png

Hi, 

There still seems to be a bit of a dark edge just outside the white line so it looks unsharp. It might just be a matter of reducing the size of the dark circle background so it's slightly smaller than the circle outline. You can just attach a screenshot for me for the next UI review. Thanks

Comment 6

[Amy Lee [:amylee] UX]

Comment on attachment 8511896 [details] [review]
patch for Gaia/master

Hi, 

There still seems to be a bit of a dark edge just outside the white line so it looks unsharp. It might just be a matter of reducing the size of the dark circle background so it's slightly smaller than the circle outline. You can just attach a screenshot for me for the next UI review. Thanks

Comment 7

[Pavel Ivanov [:ivanovpavel][:pivanov] UX]

Hey Amy,
now I reduce the size of the dark circle by 0.2rem is this enough?

Comment 8

[Amy Lee [:amylee] UX]

(In reply to Pavel Ivanov [:ivanovpavel] from comment #7)
> Hey Amy,
> now I reduce the size of the dark circle by 0.2rem is this enough?

Can you send me a screenshot? I'm still seeing a slight outline but I'm not sure if it's because my phone isn't flashing the updated patch.

Comment 9

[Pavel Ivanov [:ivanovpavel][:pivanov] UX]

Attachment: Shot

Comment 10

[Amy Lee [:amylee] UX]

Comment on attachment 8513499 [details]
Shot

Perfect! Thanks for fixing this. Can you make sure the same style is applied to the video player in the gallery app? Thanks

Comment 11

[Justin D'Arcangelo [:justindarc]]

Comment on attachment 8511896 [details] [review]
patch for Gaia/master

I have one question in the GitHub PR about one of the CSS declarations that I believe is redundant, but other than that it looks ok to me. Also, based on what Amy said in Comment 10, I'm wondering if we shouldn't just move the CSS file under "shared/style/media" since "video_player.js" originates from "shared/js/media" and is used in both Camera and Gallery. If not, you'll likely need a follow-up bug to copy/paste the same CSS changes into Gallery.

Comment 12

[Pavel Ivanov [:ivanovpavel][:pivanov] UX]

Thanks :)

I think the "shared/style/" is only for building blogs and they will be transferred to web components ... I will leave them as they are for now.

Landed to master: 
https://github.com/mozilla-b2g/gaia/commit/86b3cdf8bc50698a58e01dccd63f5c7318e02d8a