973597 - Camera "Flash Status" Label misplaced

Status: RESOLVED WONTFIX

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

Description

[Ahmed Nefzaoui [:Nefzaoui]]

Attachment: camera status label misplaced

In Portrait mode, this is how Flash Status look like.
I don't know about the UX specs but this is definitely not the right implementation..

Comment 1

[Ahmed Nefzaoui [:Nefzaoui]]

David,
What do you think? :)

Comment 2

[Diego Marcos [:dmarcos]]

This is a css issue

Comment 3

[Diego Marcos [:dmarcos]]

Attachment: Pull Request

Comment 4

[Diego Marcos [:dmarcos]]

Is the flash label gone in the UX spec for 1.4?

Comment 5

[Rob MacDonald [:robmac]]

Hi Diego... There should be a label as the user's finger can obscure the button as it toggles. I just checked the spec and it may have been accidentally got removed when I moved around some of the pages. I'll make an update to this along with HDR and send it out later today. - Rob

Comment 6

[Wilson Page [:wilsonpage]]

robmac: From the spec it appears that the flash status text should appear in a similar location as the settings confirmation text and the batter low text. If we can unify these three notifications, it will simplify the code and be more consistent/familiar UX.

Comment 7

[Rob MacDonald [:robmac]]

(In reply to Wilson Page [:wilsonpage] from comment #6)
> robmac: From the spec it appears that the flash status text should appear in
> a similar location as the settings confirmation text and the batter low
> text. If we can unify these three notifications, it will simplify the code
> and be more consistent/familiar UX.

Hi Wilson...

Thanks for flagging me and, yes, that is the intent. All of these notifications should be in the same location. 

- Rob

Comment 8

[Diego Marcos [:dmarcos]]

Attachment: Pull Request Camera New Features

Should this patch go to master instead of camera-new-features?

Comment 9

[Diego Marcos [:dmarcos]]

Attachment: Pull Request to Master

Comment 10

[Diego Marcos [:dmarcos]]

I added two PRs. One for master and the other on camera-new-features. I let djf decide which one we merge

Comment 11

[David Flanagan [:djf]]

Comment on attachment 8384906 [details] [review]
Pull Request Camera New Features

This looks like it is just the same as the previous version of the patch which prevents the label from going off the edge of the screen.

But see comments 5 and 6: the design now calls for the flash text on a completely different part of the screen.

I believe that the code we have on master just disables the flash text completely

Comment 12

[David Flanagan [:djf]]

Comment on attachment 8384910 [details] [review]
Pull Request to Master

If the intent is to fix the text that goes off the edge of the screen, I guess I'm okay with landing this on master.

But given that that would still be wrong, I think that leaving the text disabled on master and fixing it correctly on camera-new-features is the way to go.

If you really want this to land on master, let's get Rob's approval.

Comment 13

[Diego Marcos [:dmarcos]]

The text is not just disabled in master, it is rendered in the wrong location when rotating the device. It looks bad. We can move the flash label to a different location in the new design but I think we should fix this in master.

Comment 14

[Diego Marcos [:dmarcos]]

I'm also happy to drop this patch if we can live with the current rotation bug until we land the new notifications design. 

The new notifications is a more complicated change that will land as part of the Madai work.

Comment 15

[David Flanagan [:djf]]

I just flashed my nexus 4 with the latest nightly and there is no text associated with the flash.  Wilson added display:none to the CSS in a patch that landed on master right at the start of the workweek.

So it seems simpler to just not land this at all.

Comment 16

[Diego Marcos [:dmarcos]]

This wont land in master so I close the bug