Closed Bug 1168849 Opened 9 years ago Closed 7 years ago

Decorate AwesomeScreen table rows

Categories

(Firefox for iOS :: Home screen, defect)

Product:
Firefox for iOS
Component:
Home screen
Platform:
All
iOS
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Tracking Flags:
Tracking Status
fxios + ---

People

(Reporter: dhenein, Unassigned)

References

Details

(Whiteboard: [perf and accessibility impact])

Attachments

(2 files)

Bookmark badge
47 bytes, text/x-github-pull-request
rnewman
: review+
Details | Review
Pull request
48 bytes, text/x-github-pull-request
rnewman
: review-
Details | Review 
Mockups show bookmark star, "Switch to tab", synced tab icon/label where appropriate. 

Mockup: http://invis.io/ND25IXZ7T
Attached file Bookmark badge — 
This has the stuff to query the db for bookmarks, so I thought you'd be a good reviewer. The two line view stuff isn't set up extremely well for this, but I don't really want to tackle that here (i.e. making us use one view type for top sites and history/bookmark panels).
Attachment #8612101 - Flags: review?(rnewman)
Will this patch add bookmark badges to the search results rows as well (as shown in the mockup?)
Flags: needinfo?(wjohnston)
Assignee: nobody → dhenein
Status: NEW → ASSIGNED
Assignee: dhenein → wjohnston
Comment on attachment 8612101 [details] [review]
Bookmark badge

Comments on the bug.

Main open question is RTL. Also interested in how VoiceOver interacts with this. Please look into those (and my comments on the PR!) before landing.
Attachment #8612101 - Flags: review?(rnewman) → review+
tracking-fennec: ? → +
tracking-fxios: --- → +
This will have some pref impact, so I think we voted in Triage to push it to 1.1.
tracking-fennec: + → ---
status-fxios-v1.1: --- → ?
tracking-fxios: + → ---
Flags: needinfo?(wjohnston)
Just pinging you so that this doesn't get lost rnewman (the PR is probably enough to prevent that though). I'll go mark the PR as a 1.1 feature as well.
Assignee: wjohnston → nobody
Status: ASSIGNED → NEW
Flags: needinfo?(rnewman)
status-fxios-v1.1: ? → ---
tracking-fxios: --- → 1.1+
Flags: needinfo?(rnewman)
Whiteboard: [has bitrotted patch][perf and accessibility impact]
Assignee: nobody → etoop
Attached file Pull request — 
chosen mostly because you reviewed the original :wesj patch
Attachment #8657853 - Flags: ui-review?(dhenein)
Attachment #8657853 - Flags: review?(rnewman)
Status: NEW → ASSIGNED
Component: Theme & Visual Design → Home screen
Hardware: Other → All
Comment on attachment 8657853 [details] [review]
Pull request

I don't have enough confidence in the perf impact to r+ this. See comments on the PR.

I also would like to see this split into cleanup (if you still want to do it) and material changes.

You might also choose to split this bug into the badging stuff and the data level stuff. The former can be ui-reviewed and landed independently.
Attachment #8657853 - Flags: review?(rnewman) → review-
Also a note to make sure the star cell doesn't impact a11y.
Whiteboard: [has bitrotted patch][perf and accessibility impact] → [perf and accessibility impact]
Attachment #8657853 - Flags: ui-review?(dhenein)
Assignee: etoop → nobody
Status: ASSIGNED → NEW
I'll be adding a grey star as a simple accessory view, and decorating search results, in Bug 1185038.
Depends on: 1185038
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: