Closed Bug 1015841 Opened 10 years ago Closed 10 years ago

[Messages][Refresh] Recipients panel visible area handling

Categories

(Firefox OS Graveyard :: Gaia::SMS, defect)

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(feature-b2g:2.1, tracking-b2g:backlog)

RESOLVED FIXED
2.1 S1 (1aug)
feature-b2g 2.1
tracking-b2g backlog

People

(Reporter: julienw, Assigned: azasypkin)

References

(Depends on 1 open bug)

Details

(Whiteboard: [p=2])

Attachments

(1 file)

In Bug 963018, we refreshed the recipients panel without changing how the recipients panel works.

However, if the panel takes only one line, it should take less visible space than what has been done in bug 963018.

See spec: attachment 8394786 [details] and attachment 8394787 [details]

In this bug, we'll see if this is possible to do it in CSS only, and if not, we'll either abandon this requirement or implement a efficient-enough JS-based solution.
OS: Linux → Gonk (Firefox OS)
Hardware: x86_64 → ARM
Blocks: 963018
No longer depends on: 963018
Whiteboard: [p=2]
Target Milestone: --- → 2.1 S1 (1aug)
Will take it to provide JS solution
Assignee: nobody → azasypkin
Status: NEW → ASSIGNED
Comment on attachment 8464157 [details] [review]
GitHub branch URL (JS solution)

Hey Julien,

Here is WIP with JS solution, it would be great to get your feedback\suggestions as early as possible before any further improvements\investigations of this approach.

Thanks!
Attachment #8464157 - Flags: feedback?(felash)
Comment on attachment 8464157 [details] [review]
GitHub branch URL (JS solution)

feedback+ but I think we need to have something when the user is typing and moving from one line to 2 lines (before assimilating the recipient).
Attachment #8464157 - Flags: feedback?(felash) → feedback+
(In reply to Julien Wajsberg [:julienw] from comment #4)
> Comment on attachment 8464157 [details] [review]
> GitHub branch URL (JS solution, WIP)
> 
> feedback+ but I think we need to have something when the user is typing and
> moving from one line to 2 lines (before assimilating the recipient).

Thanks for feedback! ni? so that you can try it on device once you have some time. 

Thanks!
Flags: needinfo?(felash)
Yep, this works fine, and my comment 4 seems to be already working fine !
Flags: needinfo?(felash)
Depends on: 1047241
Comment on attachment 8464157 [details] [review]
GitHub branch URL (JS solution)

Hey Julien, I think it's ready for the first round of review :)

Could you please take a look (I've left a few question at github)?

Not related to this bug, but just curious: while measuring performance\reflows\repaints, I've noticed that "To:" label is always repainted on user input, but if "To:" is wrapped in additional block element it's not repainted anymore, do you know what happens here?

Thanks!
Attachment #8464157 - Flags: review?(felash)
Comment on attachment 8464157 [details] [review]
GitHub branch URL (JS solution)

I don't see any outstanding performance issue.

Some simple comments and I think we're good after this!
Attachment #8464157 - Flags: review?(felash)
Comment on attachment 8464157 [details] [review]
GitHub branch URL (JS solution)

Fixed your suggestions, please take a look again.

Thanks!
Attachment #8464157 - Attachment description: GitHub branch URL (JS solution, WIP) → GitHub branch URL (JS solution)
Attachment #8464157 - Flags: review?(felash)
Comment on attachment 8464157 [details] [review]
GitHub branch URL (JS solution)

Still one nit about the unit test: I'd prefer that we don't use "emit" from mock_recipients because it uses some custom code from the mock. I'd prefer that we use sinon's yield function with a stub so that we exercize the tested code directly.
(I admit that sinon's yield is probably more complex than the mock code, but it's common and unit tested ;) ).
Attachment #8464157 - Flags: review?(felash)
Comment on attachment 8464157 [details] [review]
GitHub branch URL (JS solution)

Thanks! Using "sinon.yield" instead of "emit" now.
Attachment #8464157 - Flags: review?(felash)
Comment on attachment 8464157 [details] [review]
GitHub branch URL (JS solution)

r=me

Please wait for a full Try run before merging :)
Attachment #8464157 - Flags: review?(felash) → review+
(In reply to Julien Wajsberg [:julienw] from comment #12)
> Comment on attachment 8464157 [details] [review]
> GitHub branch URL (JS solution)
> 
> r=me
> 
> Please wait for a full Try run before merging :)

Thanks for review! Try is green now :)

Master: https://github.com/mozilla-b2g/gaia/commit/ee3cba1c44b37e81c2ec1e79c240a1522c9ca11f
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
blocking-b2g: --- → backlog
feature-b2g: --- → 2.1
Depends on: 1050682
blocking-b2g: backlog → ---
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: