[Browser] Start page bookmark star should be greyed out

RESOLVED FIXED

Status

Firefox OS
Gaia::Browser
P3
normal
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: ibarlow, Assigned: daleharvey)

Tracking

({polish})

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [sprintready]visual design, visual-tracking, interaction c=browser u=user, leorun3, leorun4)

Attachments

(3 attachments)

(Reporter)

Description

5 years ago
Since we can't bookmark the start page, the star should be greyed out here. Either that, or we should allow the start page to be bookmarked.
Keywords: perf → polish
OS: Mac OS X → All
Hardware: x86 → All
Agreed.
Assignee: nobody → ben
Priority: -- → P3
Summary: Start page bookmark star should be greyed out → [Browser] Start page bookmark star should be greyed out
Patryk, do you want to provide an asset for the disabled state of the bookmark icon?
Flags: needinfo?(padamczyk)
Can you please use opacity of 50% for disabled action icons.
Flags: needinfo?(padamczyk)
Whiteboard: visual design, interaction → visual design, interaction c=browser u=user
Hi Ben, will you be able to implement this for v1.2?
Flags: needinfo?(bfrancis)
Whiteboard: visual design, interaction c=browser u=user → visual design, visual-tracking, interaction c=browser u=user

Comment 5

5 years ago
take it
Assignee: bfrancis → gasolin

Comment 6

5 years ago
Created attachment 755826 [details]
pull request redirect to github

add style in hide/showStartscreen to greyed out the bookmark-button
Attachment #755826 - Flags: review?(bfrancis)
Comment on attachment 755826 [details]
pull request redirect to github

Thanks for the patch Fred.

Can we use the disabled attribute instead of a "greyed" class to be consistent with the back and forward buttons, and have it set to disabled in the HTML by default so that it doesn't flash from enabled to disabled on startup?

Otherwise it seems to work well :)
Oh also, if we're going to use 0.5 opacity for the bookmark button as Patryk says, I think we should probably do that for the back button as well and remove the alternative image file because at the moment I think they're different shades of grey which looks a bit odd.

Comment 9

5 years ago
Created attachment 757218 [details]
startscreen, all button disabled with opacity 0.5

ni? patrick, 

Here is the screenshot of startscreen, all button disabled with opacity 0.5.

Are you fine with removing backbutton disabled image back-disabled.png/back-disabled@2x.png file and use opacity 0.5 instead?
Flags: needinfo?(padamczyk)

Comment 10

5 years ago
Hi ben,

Thanks for review. I've changed the PR and use 'disable' attribute to grey out bookmark button and back button to opacity 0.5.
Spoke to Ian (ibarlow) and since both icons are disabled indefinitely on that page, I think we should just remove the toolbar altogether.
Flags: needinfo?(padamczyk)

Comment 12

5 years ago
Comment on attachment 755826 [details]
pull request redirect to github

pull back review since the UI spec has changed...
Attachment #755826 - Flags: review?(bfrancis)

Comment 13

5 years ago
move to higher priority issue
Assignee: gasolin → nobody
Clearing NEEDINFO, don't think there's any open questions on me here, we just need to implement the UX change.
Flags: needinfo?(bfrancis)

Updated

5 years ago
Whiteboard: visual design, visual-tracking, interaction c=browser u=user → visual design, visual-tracking, interaction c=browser u=user, leorun3
Assignee: nobody → dale
Created attachment 765700 [details]
Pointer to GH
Attachment #765700 - Flags: review?(bfrancis)
Comment on attachment 765700 [details]
Pointer to GH

See comments in pull request https://github.com/mozilla-b2g/gaia/pull/10518
Attachment #765700 - Flags: review?(bfrancis) → review-
Comment on attachment 765700 [details]
Pointer to GH

Updated to fix startup visuals
Attachment #765700 - Flags: review- → review?(bfrancis)
Whiteboard: visual design, visual-tracking, interaction c=browser u=user, leorun3 → [sprintready]visual design, visual-tracking, interaction c=browser u=user, leorun3

Updated

5 years ago
Whiteboard: [sprintready]visual design, visual-tracking, interaction c=browser u=user, leorun3 → [sprintready]visual design, visual-tracking, interaction c=browser u=user, leorun3, leorun4
Comment on attachment 765700 [details]
Pointer to GH

r+me
Attachment #765700 - Flags: review?(bfrancis) → review+
Merged into master in https://github.com/mozilla-b2g/gaia/commit/c25e3520dda20f158980236b9f1392c0b83de992
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED

Comment 20

5 years ago
Commenting as this bug is in the list Resol-fixed leorun4 bugs to be tested. 
The start is not greyed but it is not enabled. 
Checked with latest leo build and unagi device v1-train 07/17 build:
Gecko-2d17cfb
Gaia-773a197

Would that be ok as the expected behaviour? Adding ni Ian Barlow
Flags: needinfo?(ibarlow)
(Reporter)

Comment 21

5 years ago
(In reply to Isabel Rios [:isabel_rios] from comment #20)
> Commenting as this bug is in the list Resol-fixed leorun4 bugs to be tested. 
> The start is not greyed but it is not enabled. 
> Checked with latest leo build and unagi device v1-train 07/17 build:
> Gecko-2d17cfb
> Gaia-773a197
> 
> Would that be ok as the expected behaviour? Adding ni Ian Barlow

No. We need to either 

a. Use a disabled star graphic (greyed out, like the back button), or 
b. Hide the toolbar altogether, as per patryk's suggestion in comment 11
Flags: needinfo?(ibarlow)
Just an update, this is resolved fixed, the toolbar is hidden by the patch ^^^
You need to log in before you can comment on or make changes to this bug.