Closed Bug 1096185 Opened 5 years ago Closed 5 years ago

[Text Selection] UI improvement: Adjust Caret size and shadow

Categories

(Firefox OS Graveyard :: Gaia::System::Window Mgmt, defect)

x86
macOS
defect
Not set

Tracking

(firefox36 wontfix, firefox37 wontfix, firefox38 fixed, b2g-v2.2 fixed, b2g-master fixed)

RESOLVED FIXED
2.2 S6 (20feb)
Tracking Status
firefox36 --- wontfix
firefox37 --- wontfix
firefox38 --- fixed
b2g-v2.2 --- fixed
b2g-master --- fixed

People

(Reporter: Carol, Assigned: TYLin)

References

Details

Attachments

(7 files, 8 obsolete files)

99.88 KB, image/png
Details
38.76 KB, application/zip
Details
63.73 KB, patch
TYLin
: review+
Carol
: ui-review+
Details | Diff | Splinter Review
1.19 KB, patch
automatedtester
: review+
Details | Diff | Splinter Review
3.58 KB, patch
automatedtester
: review+
jeremychen
: feedback+
Details | Diff | Splinter Review
140.20 KB, image/png
Details
139.81 KB, image/png
Details
Attached image screenshot_1110.png
Text selection's caret shadow need to sync with the pop-up menu's shadow.
Hi Ting-yu,
Could you modify the caret shadow as the same size as pop-up menu shadow, thanks!

George is in charge of the pop-up menu.
Flags: needinfo?(tlin)
Blocks: 1069288
Hi Carol,

Currently, the carets used the png images in Text_Selection_Visual Spec.zip provided in bug 1024930. I can update the carets once you give me the latest caret images with updated shadow size.
Flags: needinfo?(tlin) → needinfo?(chuang)
Attached file Caret_images_1111.zip (obsolete) —
Hi Ting-Yu,
Could you help update the care images?
Please let me have UI review when you're done! Thank you!!!
Flags: needinfo?(chuang) → needinfo?(tlin)
Flags: needinfo?(tlin)
Assignee: nobody → tlin
Status: NEW → ASSIGNED
Attached patch Adjust caret shadow. (obsolete) — Splinter Review
The sizes of the images are not changed. No need to modify ua.css.
Attachment #8521258 - Flags: ui-review?(chuang)
Attachment #8521258 - Flags: review?(roc)
Comment on attachment 8521258 [details] [diff] [review]
Adjust caret shadow.

thank you!
Attachment #8521258 - Flags: ui-review?(chuang) → ui-review+
Comment on attachment 8521258 [details] [diff] [review]
Adjust caret shadow.

Review of attachment 8521258 [details] [diff] [review]:
-----------------------------------------------------------------

rubber-stamp=me
Attachment #8521258 - Flags: review?(roc) → review+
Attached file Caret_images_1121.zip (obsolete) —
Hi Ting-Yu,
Please update the caret images from the attachment, thanks!!
Attachment #8520449 - Attachment is obsolete: true
Flags: needinfo?(chuang) → needinfo?(tlin)
Flags: needinfo?(tlin)
Summary: [Text Selection] UI improvement: Adjust Caret shadow → [Text Selection] UI improvement: Adjust Caret size and shadow
Tweak ua.css due to the size of the caret images changed.
Attachment #8521258 - Attachment is obsolete: true
Attachment #8528181 - Flags: ui-review?(chuang)
Attachment #8528181 - Flags: review?(roc)
Attachment #8528181 - Flags: ui-review?(chuang)
Attachment #8528181 - Flags: review?(roc)
Do we need still to revise the caret images?
Flags: needinfo?(ofeng)
Flags: needinfo?(chuang)
Hi Ting-Yu,
Can you flash the latest version of the text caret UI look on a device? so I can check if the size is ok.
Because the UI review you gave me was all coding.
thanks
Flags: needinfo?(chuang) → needinfo?(tlin)
We've discussed offline that the size of the caret still need to be revised.
Flags: needinfo?(tlin)
Attached file Caret_images_1121.zip (obsolete) —
Hi Ting-Yu,
Please help replace the images.
I would like to see a patch to check if the size is ok. Thank you!
Flags: needinfo?(tlin)
Hi Carol, 

The size of tilt right images are smaller than normal and tilt right ones, which seem incorrect to me. Please help confirm that. Thanks.
Flags: needinfo?(tlin)
Flags: needinfo?(ofeng)
Attached file Caret_images_0211.zip (obsolete) —
Hi Ting-Yu,
I've revised the size of the files.
Please help update it! thanks!
Flags: needinfo?(tlin)
Attached file Caret_images_0211.zip
Hi Ting-Yu
please update the images, thanks again!
Attachment #8562626 - Attachment is obsolete: true
The size of the images is enlarged by 1.5x. The CSS rules in ua.css are
changed accordingly.
Attachment #8528181 - Attachment is obsolete: true
Attachment #8563213 - Flags: ui-review?(chuang)
Attachment #8563213 - Flags: review?(roc)
Flags: needinfo?(tlin)
Attachment #8563213 - Flags: review?(roc) → review+
Attachment #8563213 - Flags: ui-review?(chuang) → ui-review+
Attachment #8563236 - Flags: feedback?(jeremychen)
Eliminate the magic number 29 and make the calculation of range start
and end explicitly. It's sad we cannot retrieve the margin left from
ua.css directly. We need to copy them to the test file by hand.
Attachment #8563236 - Attachment is obsolete: true
Attachment #8563267 - Flags: review?(dburns)
Attachment #8563267 - Flags: feedback?(jeremychen)
Due to the caret images changed, we need to change this test
accordingly.

It's sufficient to perform hit tests only targeting the left edge of the
left tilted caret and the right edge of the right tilted caret. It also
reduces the running time of the tests.

It's sad that we cannot retrieve the CSS style of the carets from ua.css
directly. We need to copy them to the test file by hand.
Attachment #8563267 - Attachment is obsolete: true
Attachment #8563267 - Flags: review?(dburns)
Attachment #8563267 - Flags: feedback?(jeremychen)
Attachment #8563304 - Flags: review?(dburns)
Attachment #8563304 - Flags: feedback?(jeremychen)
Attachment #8563304 - Flags: review?(dburns) → review+
Attachment #8563268 - Flags: review?(dburns) → review+
Comment on attachment 8563304 [details] [diff] [review]
Part 2 - Change hit test in tilt mode. (v3)

Review of attachment 8563304 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good to me. :)
Attachment #8563304 - Flags: feedback?(jeremychen) → feedback+
Attachment #8563865 - Attachment description: Screenshot for old caret 29x31.png → Screenshot for old caret 29x31
Comment on attachment 8563213 [details] [diff] [review]
Part 1 - Adjust caret size and shadow. (v2, carry roc r+)

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): None. Revise image resource by UX team request.
User impact if declined: The caret will be smaller and harder to drag.
Testing completed: Pass marionette tests on try.
Risk to taking this patch (and alternatives if risky): No risk. Simple image and CSS changes.
String or UUID changes made by this patch: None.
Attachment #8563213 - Flags: approval-mozilla-b2g37?
Patch 2 and 3 modified the test case written in bug 1130951. They won't be uplifted unless patches in bug 1130951 are also uplifted.
Attachment #8526629 - Attachment is obsolete: true
Attachment #8561220 - Attachment is obsolete: true
Attachment #8563213 - Flags: approval-mozilla-b2g37? → approval-mozilla-b2g37+
You need to log in before you can comment on or make changes to this bug.