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

Status: RESOLVED FIXED

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

Description

[Carol Huang [:Carol]]

Attachment: screenshot_1110.png

Text selection's caret shadow need to sync with the pop-up menu's shadow.

Comment 1

[Carol Huang [:Carol]]

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.

Comment 2

[Ting-Yu Lin [:TYLin] (PST, UTC-7)]

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.

Comment 3

[Carol Huang [:Carol]]

Attachment: Caret_images_1111.zip

Hi Ting-Yu,
Could you help update the care images?
Please let me have UI review when you're done! Thank you!!!

Comment 4

[Ting-Yu Lin [:TYLin] (PST, UTC-7)]

Attachment: Adjust caret shadow.

The sizes of the images are not changed. No need to modify ua.css.

Comment 5

[Carol Huang [:Carol]]

Comment on attachment 8521258 [details] [diff] [review]
Adjust caret shadow.

thank you!

Comment 6

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 8521258 [details] [diff] [review]
Adjust caret shadow.

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

rubber-stamp=me

Comment 7

[Ting-Yu Lin [:TYLin] (PST, UTC-7)]

https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=b8bf2e60f117

Comment 8

[Ting-Yu Lin [:TYLin] (PST, UTC-7)]

Hi Carol,

Since bug 1096169 was landed, we face a problem that the selection carets becomes tilt as soon the whitespace area in text_caret.png overlap. It would be great if we can make the image boundary as close to the shape of caret as possible. In this way, carets become tilt when they actually overlap.

We could enlarge the boundary by program to determine whether the user touches the caret or not. It'll still be easy for the user to drag the carets.

Comment 9

[Ting-Yu Lin [:TYLin] (PST, UTC-7)]

As for the virtual boundary enlarged by program, please give me the size in px under 1x resolution. We could use it to scale up for higher resolution such as 1.5x, 2x, etc.

Comment 10

[Carol Huang [:Carol]]

Attachment: Caret_images_1121.zip

Hi Ting-Yu,
Please update the caret images from the attachment, thanks!!

Comment 11

[Ting-Yu Lin [:TYLin] (PST, UTC-7)]

Attachment: Adjust caret size and shadow. r=roc (v2)

Tweak ua.css due to the size of the caret images changed.

Comment 12

[Ting-Yu Lin [:TYLin] (PST, UTC-7)]

Do we need still to revise the caret images?

Comment 13

[Carol Huang [:Carol]]

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

Comment 14

[Ting-Yu Lin [:TYLin] (PST, UTC-7)]

We've discussed offline that the size of the caret still need to be revised.

Comment 15

[Carol Huang [:Carol]]

Attachment: Caret_images_1121.zip

Hi Ting-Yu,
Please help replace the images.
I would like to see a patch to check if the size is ok. Thank you!

Comment 16

[Ting-Yu Lin [:TYLin] (PST, UTC-7)]

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.

Comment 17

[Carol Huang [:Carol]]

Attachment: Caret_images_0211.zip

Hi Ting-Yu,
I've revised the size of the files.
Please help update it! thanks!

Comment 18

[Carol Huang [:Carol]]

Attachment: Caret_images_0211.zip

Hi Ting-Yu
please update the images, thanks again!

Comment 19

[Ting-Yu Lin [:TYLin] (PST, UTC-7)]

Attachment: Part 1 - Adjust caret size and shadow. (v2, carry roc r+)

The size of the images is enlarged by 1.5x. The CSS rules in ua.css are
changed accordingly.

Comment 20

[Ting-Yu Lin [:TYLin] (PST, UTC-7)]

Attachment: Part 2 - Change tap test range in test_selectioncarets.py.

Comment 21

[Ting-Yu Lin [:TYLin] (PST, UTC-7)]

All tests on b2g:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=611bface4a70

Marionette tests only on all platforms:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=253f79fdeaee

Comment 22

[Ting-Yu Lin [:TYLin] (PST, UTC-7)]

Attachment: Part 2 - Change tap test range in test_selectioncarets.py. (v2)

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.

Comment 23

[Ting-Yu Lin [:TYLin] (PST, UTC-7)]

Attachment: Part 3 - Fix function redefinition in test_selectioncarets.py. (v1)

Comment 24

[Ting-Yu Lin [:TYLin] (PST, UTC-7)]

Attachment: Part 2 - Change hit test in tilt mode. (v3)

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.

Comment 25

[Jeremy Chen ([e-mail])]

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. :)

Comment 26

[Ting-Yu Lin [:TYLin] (PST, UTC-7)]

Try run:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=6fbefb718dc2

Comment 27

[Ting-Yu Lin [:TYLin] (PST, UTC-7)]

Attachment: Screenshot for old caret 29x31

Comment 28

[Ting-Yu Lin [:TYLin] (PST, UTC-7)]

Attachment: Screenshot for new caret 44x47

Comment 29

[Ting-Yu Lin [:TYLin] (PST, UTC-7)]

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.

Comment 30

[Ting-Yu Lin [:TYLin] (PST, UTC-7)]

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.

Comment 31

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/b2g-inbound/rev/f0f528da4d3b
https://hg.mozilla.org/integration/b2g-inbound/rev/eb7e4b1b9cf8
https://hg.mozilla.org/integration/b2g-inbound/rev/3563d3cfba5c

Comment 32

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/f0f528da4d3b
https://hg.mozilla.org/mozilla-central/rev/eb7e4b1b9cf8
https://hg.mozilla.org/mozilla-central/rev/3563d3cfba5c

Comment 33

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/089444e5358e