[Messages] Compose Panel refresh (subject behavior change)

RESOLVED FIXED

Status

defect
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: julienw, Assigned: azasypkin)

Tracking

unspecified
ARM
Gonk (Firefox OS)
Dependency tree / graph

Firefox Tracking Flags

(feature-b2g:2.2+, tracking-b2g:backlog)

Details

(Whiteboard: [p=2][sms-sprint-2.1S3])

Attachments

(1 attachment)

46 bytes, text/x-github-pull-request
steveck
: review+
steveck
: feedback+
Details | Review
Reporter

Description

5 years ago
See attachment 8461328 [details] for the UX spec.
See attachment 8465323 [details] for the design spec.
Reporter

Updated

5 years ago
Reporter

Updated

5 years ago
See Also: → 1048362
Per product prioritization, this could be 2.2 focus.
blocking-b2g: --- → backlog
feature-b2g: --- → 2.1
Reporter

Comment 2

5 years ago
Wesley, I think you chose the wrong value for feature-b2g.
Flags: needinfo?(whuang)
You're right. Thanks for pointing this out.
feature-b2g: 2.1 → 2.2
Flags: needinfo?(whuang)
Posted file GitHub branch URL
Here is WIP patch on that I have for some time already, will polish it if we have time during v2.1 timeframe, otherwise - v2.2
Comment on attachment 8470839 [details] [review]
GitHub branch URL

Hey Steve,

Here is the 2.1 VR for the subject field. As you know, it's postponed to v2.2 only, so there is no rush here. But anyway it would be great to get your early feedback on this when you have some time.

Thanks!
Attachment #8470839 - Flags: feedback?(schung)
Comment on attachment 8470839 [details] [review]
GitHub branch URL

Patch works really great, thanks! I still left some comments on githud and you could take a look when you have time on this :)
Attachment #8470839 - Flags: feedback?(schung) → feedback+
Comment on attachment 8470839 [details] [review]
GitHub branch URL

Hey Steve, I've mode fixes related per your comments, please take a look once again once you have time :)

Thanks!
Attachment #8470839 - Attachment description: GitHub branch URL (WIP) → GitHub branch URL
Attachment #8470839 - Flags: review?(schung)
Assignee: nobody → azasypkin
Status: NEW → ASSIGNED
OS: Mac OS X → Gonk (Firefox OS)
Hardware: x86 → ARM
Whiteboard: [p=2] → [p=2][sms-sprint-2.1S3]
Use feature-b2g:2.2? rather than just 2.2.
feature-b2g: 2.2 → 2.2?
No longer blocks: sms-refresh-v2.1
Any chance we could do an a11y review on this before it lands? It will avoid us from having to do a spinoff.
Comment on attachment 8470839 [details] [review]
GitHub branch URL

(In reply to Eitan Isaacson [:eeejay] from comment #9)
> Any chance we could do an a11y review on this before it lands? It will avoid
> us from having to do a spinoff.

I believe yes!
Attachment #8470839 - Flags: a11y-review?(eitan)
Comment on attachment 8470839 [details] [review]
GitHub branch URL

This patch gets rid of the font-size 0 hack for MMS, which is great.

I added some comments in the PR regarding roles. I think a followup bug will be required because there are stuff outside of the scope of this patch, namely:
 - Recipients field is not accessible.
 - "Message" placeholder messes with the screen reader. Instead of display block/none, it should be visibility visible/hidden. This will avoid a reflow that throws the screen reader off after entering the first character.
Attachment #8470839 - Flags: a11y-review?(eitan)
(In reply to Eitan Isaacson [:eeejay] from comment #11)
> Comment on attachment 8470839 [details] [review]
> GitHub branch URL
> 
> This patch gets rid of the font-size 0 hack for MMS, which is great.
> 
> I added some comments in the PR regarding roles. I think a followup bug will
> be required because there are stuff outside of the scope of this patch,
> namely:
>  - Recipients field is not accessible.
>  - "Message" placeholder messes with the screen reader. Instead of display
> block/none, it should be visibility visible/hidden. This will avoid a reflow
> that throws the screen reader off after entering the first character.

Thanks for review! I've updated PR with textbox roles as you pointed out. For form[role=search] I think it would be better to file another bug as it requires a bunch of unrelated to this patch style changes + one new locale string for send button aria label.
Comment on attachment 8470839 [details] [review]
GitHub branch URL

Just a concern about the message counter label, but we could also revisit this in follow up bug. But if you have a good idea now and prefer to proper a solution in this patch, please request review again, thanks.
Attachment #8470839 - Flags: review?(schung) → review+
Blocks: 1061215
(In reply to Steve Chung [:steveck] from comment #13)
> Comment on attachment 8470839 [details] [review]
> GitHub branch URL
> 
> Just a concern about the message counter label, but we could also revisit
> this in follow up bug. But if you have a good idea now and prefer to proper
> a solution in this patch, please request review again, thanks.

Thanks for review Steve! Agree that we need to improve the way we handle remaining chars counter. I've managed to do a draft solution and see that it's a good candidate for the separate bug. So I've filed one - bug 1061215.
Master: https://github.com/mozilla-b2g/gaia/commit/edfd5c48b540b08bbc4959c3c7fcc8bc133015aa
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
let's keep the already-landed improvement in 2.2.
feature-b2g: 2.2? → 2.2+
blocking-b2g: backlog → ---
You need to log in before you can comment on or make changes to this bug.