[Stingray][smart-system] Apply visual to contextmenu

RESOLVED FIXED in 2.2 S3 (9jan)

Status

Firefox OS
Gaia
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: rexboy, Assigned: rexboy)

Tracking

unspecified
2.2 S3 (9jan)
x86
Mac OS X
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [ft:conndevices][ETA: 1/9])

Attachments

(1 attachment, 1 obsolete attachment)

46 bytes, text/x-github-pull-request
johnhu
: review+
Details | Review | Splinter Review
We need to apply visual spec to contextmenu.
Things to do:

1. sense current focused cursor and change its style programmatically
2. remove selection border
3. apply visual spec
Created attachment 8529669 [details] [review]
Patch

This patch applies visual spec to context menu, but with a temporary solution:
*  The icons of context menu are svg files. We need to change them to TTF fonts after they're ready.

And it also changes behavior of shared css a little bit:
*  each included css file under tv_shared/style is allowed to have one
   same-named directory. All contents under that are copied together with css.

I need the behavior above to bring some shared icons into app.

John may you review the patch? I need to confirm if you also feel this solution is ok.
Attachment #8529669 - Flags: review?(im)
Comment on attachment 8529669 [details] [review]
Patch

I give r+ for the "JS" parts. As to css, I feel visual may have opinions on it. We should show the result to Scott and discuss with him.

We should not use ttf in this case because the menu tag is a formal tag in w3c spec. It only supports label for text and icon for image url. If we want to use icon-font, we may break this spec and introduce new attribute to it. This violates the thought of not segmenting web. IMO, we should ask visual to give us png in these cases.
Attachment #8529669 - Flags: review?(im) → review+
After discussing with Scott, the buttons need to be scrollable if there are too many options.
I'll add it to the patch.

Updated

4 years ago
Target Milestone: --- → 2.2 S3 (9jan)

Updated

4 years ago
Blocks: 1115611

Updated

4 years ago
Blocks: 1067781
No longer depends on: 1067781
Whiteboard: [ft:conndevices] → [ft:conndevices][ETA: 1/9]
Created attachment 8545761 [details] [review]
patch v2

I made several changes including:
- Use png rather than svg
- Change PNG color by invert (temporarily)
- use Scrollable so we can scroll context menu when there are too many items.
- Items are centered if they don't exceed screen width.

Since there are much changes John may you do the review again? you may just see the second commit. Thank you!
Attachment #8529669 - Attachment is obsolete: true
Attachment #8545761 - Flags: review?(im)
Comment on attachment 8545761 [details] [review]
patch v2

Looks good to me. I would suggest we file a bug to create a type of smart-button for option menu and use it here.
Attachment #8545761 - Flags: review?(im) → review+
https://github.com/mozilla-b2g/gaia/commit/d03cc5bfa746f5a09e02677fa2bafb0b5916285d

Thanks for reviewing!
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Blocks: 1119767
You need to log in before you can comment on or make changes to this bug.