If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

[Camera] 1.4 Visual Design Settings Menu

RESOLVED FIXED in Firefox OS v1.4

Status

Firefox OS
Gaia::Camera
P1
normal
RESOLVED FIXED
4 years ago
3 years ago

People

(Reporter: amylee, Assigned: wilsonpage)

Tracking

unspecified
1.4 S5 (11apr)
ARM
Gonk (Firefox OS)

Firefox Tracking Flags

(tracking-b2g:backlog, b2g-v1.4 fixed, b2g-v2.0 fixed)

Details

(Whiteboard: ux-tracking, visual design, jian [fxos:media])

Attachments

(4 attachments, 3 obsolete attachments)

(Reporter)

Description

4 years ago
Settings menu for camera.

Will attach visual specs
(Reporter)

Updated

4 years ago
blocking-b2g: --- → 1.4?
(Assignee)

Updated

4 years ago
Assignee: nobody → wilsonpage
(Reporter)

Comment 1

4 years ago
Created attachment 8378631 [details]
CSS_Pixel_Camera_Settings_Menu_Spec.png
(Reporter)

Comment 2

4 years ago
Created attachment 8379087 [details]
Camera_Settings_Menu_Part_2.png

Updated spec for secondary menu items/confirmation message

Comment 3

4 years ago
Created attachment 8380164 [details]
Pointer to Pull Request.html

Hi David,

We have implemented setting menu close in different scenarios.
This code does not include test code.
Attachment #8380164 - Flags: review?(dflanagan)
VD enhancements won't block, so not blocking
blocking-b2g: 1.4? → backlog
Comment on attachment 8380164 [details]
Pointer to Pull Request.html

Clearing the review request and setting feedback+ because this looks good to me.

A few notes:

1) Please figure out a way to not change the file modes.

2) I think you should file a bug specific to closing the settings menu and attach this patch to that new bug.  This bug is for the overall visual design of the settings menu, and that's not what you're doing here, so it is the wrong bug #.
Attachment #8380164 - Flags: review?(dflanagan) → feedback+
(Reporter)

Comment 6

4 years ago
Created attachment 8384883 [details]
CSS_Pixels_Camera_Settings_Menu.png

Hi, 

Here is a revised settings menu spec. The settings menu is now full screen to be more flexible in different situations/aspect ratios.
Attachment #8378631 - Attachment is obsolete: true
(Reporter)

Comment 7

4 years ago
Hi Wilson, 

I remember we had worked on tweaking the style in the settings menu. I don't see the changes in the camera-new-features branch. Is it somewhere else? 

Thanks
Flags: needinfo?(wilsonpage)
(Assignee)

Comment 8

4 years ago
These style tweaks are in camera-new-features. Notice the 'FiraSansLight' font on the main menu titles? Plus other things.

Has something got lost?
Flags: needinfo?(wilsonpage)
(Reporter)

Comment 9

4 years ago
(In reply to Wilson Page [:wilsonpage] from comment #8)
> These style tweaks are in camera-new-features. Notice the 'FiraSansLight'
> font on the main menu titles? Plus other things.
> 
> Has something got lost?

Hi Wilson, 

Yeah I think a few things got lost. Can you make the following adjustments? 

The 2nd line white text should be Fira Sans Medium / 1.7 rem. If we really can’t have the actual font, please make this font-weight: 500

Can you check the vertical alignment on the icon and the text block? It should be vertically aligned to the row but it looks like it’s sitting a bit too low.

Please add a hyphen to Self Timer so it reads Self-Timer. 

Thanks!
(Assignee)

Updated

4 years ago
Attachment #8380164 - Attachment is obsolete: true
(Assignee)

Comment 10

4 years ago
Created attachment 8395398 [details] [review]
pull-request (camera-new-features)
Attachment #8395398 - Flags: ui-review?(amlee)
In the specs there's too little margin between the icon and the left border of the setting option.

Also there's no consistency on the active state color for the settings options and the rest of the camera UI. I had created bug 986882 already has a PR to fix that.
Flags: needinfo?(amlee)
(Reporter)

Comment 12

4 years ago
(In reply to Diego Marcos [:dmarcos] from comment #11)
> In the specs there's too little margin between the icon and the left border
> of the setting option.
> 
> Also there's no consistency on the active state color for the settings
> options and the rest of the camera UI. I had created bug 986882 already has
> a PR to fix that.

Hi Diego, 

The highlight state is based off of our existing style guide for dark apps http://www.mozilla.org/en-US/styleguide/products/firefox-os/lists/. Also, the active selection in the menu is currently the same shade of blue so if you make on press state the same transparency, you no longer see the text and check mark when you highlight it. Please leave as it. 

The spacing between the edge of the icon and the screen is correct. Please leave it as is.
Flags: needinfo?(amlee)
(Reporter)

Comment 13

4 years ago
(In reply to Amy from comment #12)
> (In reply to Diego Marcos [:dmarcos] from comment #11)
> > In the specs there's too little margin between the icon and the left border
> > of the setting option.
> > 
> > Also there's no consistency on the active state color for the settings
> > options and the rest of the camera UI. I had created bug 986882 already has
> > a PR to fix that.
> 
> Hi Diego, 
> 
> The highlight state is based off of our existing style guide for dark apps
> http://www.mozilla.org/en-US/styleguide/products/firefox-os/lists/. Also,
> the active selection in the menu is currently the same shade of blue so if
> you make on press state the same transparency, you no longer see the text
> and check mark when you highlight it. Please leave as it. 
> 
> The spacing between the edge of the icon and the screen is correct. Please
> leave it as is.

Hi Diego, 

I just checked your patch. I see that you inverted the text to white on press so that works better. After reviewing the guidelines, it's hard to determine what category this menu falls under since it's unique to the OS so I'm okay with keeping the new highlight state. Can you please remove the drop shadows on the text and icons? 

Thanks
Flags: needinfo?(dmarcos)
(Reporter)

Comment 14

4 years ago
Created attachment 8395710 [details]
settings_screen.png

Hi Wilson, 

Can you please change the 2nd lines in the settings menu back to 1.6 rem and keep the bold. Also please make the sub menu text 1.6 rem and the same bold as well. 

I've also noticed that the rows are still not vertically aligned. There's more space at the top of the row than at the bottom. See attached. Thanks!
(Reporter)

Updated

4 years ago
Flags: needinfo?(wilsonpage)
(Assignee)

Comment 15

4 years ago
amlee: Should all be fixed now, we almost good to go?
Flags: needinfo?(wilsonpage)
(Reporter)

Comment 16

4 years ago
Hi Wilson, 

Here is my feedback:

HRD - Wrong icon, should have a white square behind “HDR”

Icons are sitting a bit too low (off by 1px) (vertically center it to row)

Check alignment in sub-headers (text is sitting a bit too low in the row)

Last row in settings and sub settings menu is 1px too wide (bump last hairline up 1px)

Check to see if margins (space between hairline and edge of screen is 15px)

Thanks!
(Reporter)

Comment 17

4 years ago
Hi Wilson, 

I've reviewed the latest patch and here is my feedback:

Move settings ICONS 1px up (portrait view only)

Move settings menu TEXT and sub-menu text 1px down (landscape view only)

Thanks!
(Assignee)

Comment 18

4 years ago
amlee: Why are these requirements orientation specific? Is there not a fix that will address all cases...?
Flags: needinfo?(amlee)
(Reporter)

Comment 19

4 years ago
(In reply to Wilson Page [:wilsonpage] from comment #18)
> amlee: Why are these requirements orientation specific? Is there not a fix
> that will address all cases...?

Hi Wilson, 

I would assume the alignment should be the same in landscape/portrait but when you overlap screenshots they are a bit off between the two.
Flags: needinfo?(amlee)
(Reporter)

Updated

4 years ago
Attachment #8395398 - Flags: ui-review?(amlee) → ui-review+
(Assignee)

Comment 20

4 years ago
Created attachment 8398050 [details] [review]
pull-request (master)
Attachment #8395398 - Attachment is obsolete: true
Attachment #8398050 - Flags: review?(dmarcos)
Attachment #8398050 - Flags: review?(dmarcos) → review+
Flags: needinfo?(dmarcos)
(Assignee)

Comment 21

4 years ago
Landed on 'master': https://github.com/mozilla-b2g/gaia/commit/632c6746a5124d852b2fd7c0b77f207e2d54ff8d
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
(Reporter)

Updated

4 years ago
Attachment #8395398 - Flags: ui-review?(tshakespeare)

Comment 22

4 years ago
The ui-review? was set on an obsolete patch. Tiff, please review what has landed on master instead. Ui-review? should come from devs, be set to interaction designers, and be completed before landing to master. Since that was not the case here, we'll just review on master.
Comment on attachment 8395398 [details] [review]
pull-request (camera-new-features)

Looking at Master I see a few issues in regards to the strings...

1. "Grid" is not to interaction spec. Spec calls for "Frame Grid" though if we could use "Grid Lines" instead that would be awesome.

2. Options (e.g. On, 5 seconds) should be sentence case, not uppercase. This is true for both the top-level menu and sub-menus.

3. Camera and video resolution should not appear in menu. I believe Wilson opened a bug for this though? 
3a. FYI for Madai build - for the camera/video sub-menu, menu title should be title case (e.g. Camera Resolution) like in the top-level menu.
Attachment #8395398 - Flags: ui-review?(tshakespeare) → ui-review-
Flags: needinfo?(wilsonpage)
Reopening based on review - above. Please fix the strings. 

Also this bug may be a dup now: https://bugzilla.mozilla.org/show_bug.cgi?id=988108
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Sorry for the many emails

Just informed of Wilson's bug for resolutions in the menu: Bug 990285 - [Camera] Resolutions should not be shown in settings menu
Flags: needinfo?(wilsonpage)
(Assignee)

Comment 26

4 years ago
Closing this bug. Additional string changes will land with bug 988108. Please move discussion over there :)
Status: REOPENED → RESOLVED
Last Resolved: 4 years ago4 years ago
Resolution: --- → FIXED
Bulk edit for camera bugs.

If earlier comments do not show how this bug landed to master, it probably landed as part of https://github.com/mozilla-b2g/gaia/pull/17599 which merged the camera-new-features branch into master.

This bug was uplifted from master to v1.4 as part of https://github.com/mozilla-b2g/gaia/commit/a8190d08e61316a86bba572ba8d894d081a20530
status-b2g-v1.4: --- → fixed
status-b2g-v2.0: --- → fixed
Target Milestone: --- → 1.4 S5 (11apr)
blocking-b2g: backlog → ---
tracking-b2g: --- → backlog
You need to log in before you can comment on or make changes to this bug.