Closed
Bug 963018
Opened 11 years ago
Closed 11 years ago
[Messages][Refresh] Update styles of the Composer
Categories
(Firefox OS Graveyard :: Gaia::SMS, defect)
Tracking
(feature-b2g:2.0, tracking-b2g:backlog, b2g-v2.0 fixed)
VERIFIED
FIXED
2.0 S3 (6june)
Tracking | Status | |
---|---|---|
b2g-v2.0 | --- | fixed |
People
(Reporter: borjasalguero, Assigned: joan)
References
Details
Attachments
(20 files)
33.25 KB,
image/png
|
Details | |
46.17 KB,
image/png
|
Details | |
35.10 KB,
image/png
|
Details | |
32.81 KB,
image/png
|
Details | |
31.86 KB,
image/png
|
Details | |
46 bytes,
text/x-github-pull-request
|
Details | Review | |
46 bytes,
text/x-github-pull-request
|
julienw
:
review+
|
Details | Review |
235.57 KB,
application/zip
|
vicky
:
ui-review-
|
Details |
19.85 KB,
image/png
|
Details | |
31.10 KB,
image/png
|
Details | |
18.86 KB,
image/png
|
Details | |
18.44 KB,
image/png
|
Details | |
164.14 KB,
image/png
|
Details | |
27.57 KB,
image/png
|
Details | |
29.49 KB,
image/png
|
Details | |
88.78 KB,
image/png
|
Details | |
25.80 KB,
image/png
|
vicky
:
ui-review+
|
Details |
29.07 KB,
image/png
|
vicky
:
ui-review+
|
Details |
25.81 KB,
image/png
|
vicky
:
ui-review+
|
Details |
29.14 KB,
image/png
|
vicky
:
ui-review+
|
Details |
The goal of this bug is to update the styles of the Composer. The markup will be modified in bug https://bugzilla.mozilla.org/show_bug.cgi?id=949457 .
This probably needs some tweaks in Building Blocks as well.
Reporter | ||
Updated•11 years ago
|
Assignee: nobody → joan.leon
Target Milestone: --- → 1.3 C3/1.4 S3(31jan)
Updated•11 years ago
|
blocking-b2g: --- → 1.4?
Reporter | ||
Comment 1•11 years ago
|
||
Hi Julien, I'm removing the dependency here because after talking with Joan we are going to do the refactoring in 2 steps:
- Fist one (this one) is *only* related with styles that does not affect the current composer (I mean markup and JS). This is only related with a refresh of the colors, fonts... That's why it's not blocked by the refactoring of the markup, and the deadline is the 31/1
- As second step, we are going to work on creating the new markup, and modify the JS related with the composer working quite close to Ayman, for ensuring that this refactor follows all requirements in the WF. The goal for this task (is a complicated one) it's 14/2.
I hope it helps!
No longer depends on: 949457
Comment 3•11 years ago
|
||
Comment 4•11 years ago
|
||
Comment 5•11 years ago
|
||
Comment 6•11 years ago
|
||
Comment 7•11 years ago
|
||
Comment 8•11 years ago
|
||
Joan asked me about these cases (MMS indicator & character counter), I did the corresponding visual for this elements in the visual refresh.
Assignee | ||
Comment 9•11 years ago
|
||
Attachment #8370122 -
Flags: review?(borja.bugzilla)
Comment 10•11 years ago
|
||
Several comments: https://github.com/mozilla-b2g/gaia/pull/15947/files
Assignee | ||
Comment 11•11 years ago
|
||
Hi Rick, I applied the corrections
Comment 12•11 years ago
|
||
Joan, is it WIP or not?
If it's WIP then it's not ready for review, right ? :)
But anyway thanks for your work!
Assignee | ||
Comment 13•11 years ago
|
||
Hi Julien,
yes it's WIP and the review is assigned to Borja, because he work with me on visual refresh.
I talk with Borja
Reporter | ||
Comment 14•11 years ago
|
||
Comment on attachment 8370122 [details] [review]
WIP, SMS Compose patch
This bug needs a new PR attached, and It has been reduced in priority. Let me know when we are back on this!
Attachment #8370122 -
Flags: review?(borja.bugzilla)
Comment 15•11 years ago
|
||
VD refresh issues are not blockers for the release, so not blocking.
blocking-b2g: 1.4? → backlog
Updated•11 years ago
|
Target Milestone: 1.3 C3/1.4 S3(31jan) → ---
Assignee | ||
Comment 16•11 years ago
|
||
Attachment #8403473 -
Flags: review?(borja.bugzilla)
Assignee | ||
Comment 17•11 years ago
|
||
Attachment #8403475 -
Flags: ui-review?(vpg)
Reporter | ||
Updated•11 years ago
|
Attachment #8403473 -
Flags: review?(felash)
Comment 19•11 years ago
|
||
Comment on attachment 8403475 [details]
VR_Message_Composer.zip
Thanks. A few comments though:
1. Can we please change hitstates to #b2f2ff?
2. Charachters left indicator should not be italic
3. Put "MMS" indicator in the same place an color of charachter indicator
4. Input text looks bold, check that it is regular style (placeholeder text also looks bolder than it should be)
5. Align file thumbnail in input area to the cursor baseline
6. Make suggestions background same color as "to field" area.
Attachment #8403475 -
Flags: ui-review?(vpg) → ui-review-
Reporter | ||
Comment 20•11 years ago
|
||
This shows the style with more than one line. Style is not working as expected.
Reporter | ||
Comment 21•11 years ago
|
||
Banner is not aligned with the bottom of the recipients layer.
Reporter | ||
Comment 22•11 years ago
|
||
Comment on attachment 8403473 [details] [review]
Path in github
I've found some issues:
- Banner: https://bugzilla.mozilla.org/attachment.cgi?id=8403966
- Recipients alignment: https://bug963018.bugzilla.mozilla.org/attachment.cgi?id=8403965
- Some CSS styles in 'px' instead of 'rem'.
Could you take a look? Ask me to take a look again when ready! THanks!
Attachment #8403473 -
Flags: review?(borja.bugzilla)
Comment 23•11 years ago
|
||
Victoria, for accessibility reason, I'm worried that the "invalid" state for a recipient is conveyed only with colors instead of pictogram like we had before. Someone that do not see colors will have a lot of difficulties to see this. This is something I raised to José when he did his first proposal too, and that's why he came up with using a "!" in addition to colors.
Flags: needinfo?(vpg)
Comment 24•11 years ago
|
||
Comment on attachment 8403473 [details] [review]
Path in github
Not much more than what Borja said, except that the recipient line, when empty, is not well vertically centered.
I'm also concerned about the invalid recipients design (I didn't raise this earlier because I couldn't really point what was disturbing me, and I realized only now, so sorry about this :( ).
Attachment #8403473 -
Flags: review?(felash)
Comment 25•11 years ago
|
||
(In reply to Julien Wajsberg [:julienw] from comment #23)
> Victoria, for accessibility reason, I'm worried that the "invalid" state for
> a recipient is conveyed only with colors instead of pictogram like we had
> before. Someone that do not see colors will have a lot of difficulties to
> see this. This is something I raised to José when he did his first proposal
> too, and that's why he came up with using a "!" in addition to colors.
Hi Julien,
I know about that conversation, and I think color blindness is not as bad as we some times put it. But in any case, there's no harm in putting back the "!" sign.
JOan is already on it. Find attached the new version of this field.
Flags: needinfo?(vpg)
Comment 26•11 years ago
|
||
Comment 27•11 years ago
|
||
Assignee | ||
Comment 28•11 years ago
|
||
(In reply to Julien Wajsberg [:julienw] from comment #24)
> Comment on attachment 8403473 [details] [review]
> Path in github
>
> Not much more than what Borja said, except that the recipient line, when
> empty, is not well vertically centered.
you mean the caret cursor?
> I'm also concerned about the invalid recipients design (I didn't raise this
> earlier because I couldn't really point what was disturbing me, and I
> realized only now, so sorry about this :( ).
Comment 29•11 years ago
|
||
(In reply to Joan Leon [:joan] from comment #28)
> (In reply to Julien Wajsberg [:julienw] from comment #24)
> > Comment on attachment 8403473 [details] [review]
> > Path in github
> >
> > Not much more than what Borja said, except that the recipient line, when
> > empty, is not well vertically centered.
>
> you mean the caret cursor?
>
No, I mean the whole line is not verically aligned inside the grey zone.
(In reply to Victoria Gerchinhoren [:vicky] from comment #25)
> But in any case, there's no harm in putting back the "!" sign.
Thanks !
Updated•11 years ago
|
Target Milestone: --- → 1.4 S6 (25apr)
Assignee | ||
Updated•11 years ago
|
Attachment #8403473 -
Flags: review?(borja.bugzilla)
Reporter | ||
Comment 30•11 years ago
|
||
Comment on attachment 8403473 [details] [review]
Path in github
Adding Julien as well! Waiting until the PR will be rebased for adding my comments.
Attachment #8403473 -
Flags: review?(felash)
Updated•11 years ago
|
Target Milestone: 1.4 S6 (25apr) → 2.0 S1 (9may)
Comment 31•11 years ago
|
||
Borja, is this ready ?
Assignee | ||
Updated•11 years ago
|
Attachment #8403473 -
Flags: ui-review?(vpg)
Updated•11 years ago
|
Target Milestone: 2.0 S1 (9may) → 1.4 S6 (25apr)
Comment 32•11 years ago
|
||
Comment on attachment 8403473 [details] [review]
Path in github
I revised the UI with Joan offline (side by side) looks OK. Thanks Joan for the hard work!
Attachment #8403473 -
Flags: ui-review?(vpg) → ui-review+
Updated•11 years ago
|
Target Milestone: 1.4 S6 (25apr) → 2.0 S1 (9may)
Reporter | ||
Comment 33•11 years ago
|
||
Comment on attachment 8403473 [details] [review]
Path in github
I think this is a great step forward, but needs some work yet. There are some scenarios (overall when the phone number or name of the recipient produces an overflow), when the UI is not working as we expected from the visual refresh.
In this case, probably there is some JS which needs to be modified, or CSS should work in a different manner. If you need help with the JS dont hesitate to ask me! I'll help you for sure! :)
Attachment #8403473 -
Flags: ui-review+
Attachment #8403473 -
Flags: review?(borja.bugzilla)
Attachment #8403473 -
Flags: review-
Updated•11 years ago
|
QA Contact: lolimartinezcr
Comment 34•11 years ago
|
||
Victoria, the spec doesn't say where the character counter should be if the user scrolls the message. After bug 949457 lands, the subject and the message scroll together (as it should have been since the start), and so I don't know how we should align the "MMS" indicator. Could you please help?
Flags: needinfo?(vpg)
Comment 35•11 years ago
|
||
Comment on attachment 8403473 [details] [review]
Path in github
Gave more comments on github
thanks !
Attachment #8403473 -
Flags: review?(felash) → review-
Comment 36•11 years ago
|
||
Flags: needinfo?(vpg)
Comment 37•11 years ago
|
||
Note that I uploaded spec for the Subject's different scenarios.
Comment 38•11 years ago
|
||
Victoria, sorry, it's still not clear to me.
Again, now, the subject and the message text scroll together, in a "combined" way. So that means that if you scroll down, the subject goes out of the composer and is invisible. Should the "MMS" indicator scroll with the subject, or should it stay in the same place?
And another question :) If there are some pictures in the message (so it's a MMS), but there is no subject, I expect the "MMS" indicator to be aligned with the top of the composer (like the character counter is). But if there are some pictures and the user toggles the subject without writing anything in the subject yet; should the "MMS" indicator be aligned with the subject-to-be, or should it stay aligned with the text/pictures?
Flags: needinfo?(vpg)
Assignee | ||
Comment 39•11 years ago
|
||
Hi Julien,
Can we define the theme of subject as another bug (that blocks this bug)?
Flags: needinfo?(felash)
Comment 41•11 years ago
|
||
(In reply to Julien Wajsberg [:julienw] (away until 5th May) from comment #38)
> Victoria, sorry, it's still not clear to me.
>
> Again, now, the subject and the message text scroll together, in a
> "combined" way. So that means that if you scroll down, the subject goes out
> of the composer and is invisible. Should the "MMS" indicator scroll with the
> subject, or should it stay in the same place?
Really? The subject goes off screen? I thought it would stick to the top when reaching it and the message would scroll behind it, this sounds more logical to me. But we can always ask IA to give extra input here.
So NI Omega
>
> And another question :) If there are some pictures in the message (so it's a
> MMS), but there is no subject, I expect the "MMS" indicator to be aligned
> with the top of the composer (like the character counter is). But if there
> are some pictures and the user toggles the subject without writing anything
> in the subject yet; should the "MMS" indicator be aligned with the
> subject-to-be, or should it stay aligned with the text/pictures?
I think in that case, the best is that the MMS indicator stays in the same place with a sorter line until the user types something in the subject field, when the user starts writing, the line will grow to reach the undelining lenght.
Flags: needinfo?(vpg) → needinfo?(ofeng)
Comment 42•11 years ago
|
||
(In reply to Victoria Gerchinhoren [:vicky] from comment #41)
> (In reply to Julien Wajsberg [:julienw] (away until 5th May) from comment
> #38)
> > Victoria, sorry, it's still not clear to me.
> >
> > Again, now, the subject and the message text scroll together, in a
> > "combined" way. So that means that if you scroll down, the subject goes out
> > of the composer and is invisible. Should the "MMS" indicator scroll with the
> > subject, or should it stay in the same place?
>
> Really? The subject goes off screen? I thought it would stick to the top
> when reaching it and the message would scroll behind it, this sounds more
> logical to me.
This was the previous behavior but Ayman prefered to scroll the subject along with the message; I think the rationale is that if the user wants to scroll down, he wants to see more of the message.
Comment 43•11 years ago
|
||
(In reply to Victoria Gerchinhoren [:vicky] from comment #41)
> Really? The subject goes off screen? I thought it would stick to the top
> when reaching it and the message would scroll behind it, this sounds more
> logical to me. But we can always ask IA to give extra input here.
> So NI Omega
Yeah, I think it's better for subject to go off screen to leave more space for message input. The subject is not that important to be always visible.
Flags: needinfo?(ofeng)
Comment 44•11 years ago
|
||
(In reply to Omega Feng [:Omega] from comment #43)
> (In reply to Victoria Gerchinhoren [:vicky] from comment #41)
> > Really? The subject goes off screen? I thought it would stick to the top
> > when reaching it and the message would scroll behind it, this sounds more
> > logical to me. But we can always ask IA to give extra input here.
> > So NI Omega
>
> Yeah, I think it's better for subject to go off screen to leave more space
> for message input. The subject is not that important to be always visible.
Yes, I agree that subject is not that important, but for me the important thing is to show the user how adding subject to a regular SMS will turn it into a MMS being a much more expensive type of message in our target countries. So the idea of the MMS indicator to be next to the subject when there is so, is just to somehow advice -with a visual connection of elements- that one of the reasons of being an MMS is that Subject addition (a user already knows that adding a file will cost more, the second situation of having a really long message turning into an MMS is already communicated by an in app message and this third way to convert the message into an MMS is the one left to be indicated).
So if that whole element can stay there, it would be a good solution. If you can think of a better one, I am open, but would like to have that connection somehow.
Thanks!
Comment 45•11 years ago
|
||
Created the visual mockup of the new MMS in-app message
Comment 46•11 years ago
|
||
Comment 47•11 years ago
|
||
Comment 48•11 years ago
|
||
(In reply to Victoria Gerchinhoren [:vicky] from comment #44)
> (In reply to Omega Feng [:Omega] from comment #43)
> Yes, I agree that subject is not that important, but for me the important
> thing is to show the user how adding subject to a regular SMS will turn it
> into a MMS being a much more expensive type of message in our target
> countries. So the idea of the MMS indicator to be next to the subject when
> there is so, is just to somehow advice -with a visual connection of
> elements- that one of the reasons of being an MMS is that Subject addition
> (a user already knows that adding a file will cost more, the second
> situation of having a really long message turning into an MMS is already
> communicated by an in app message and this third way to convert the message
> into an MMS is the one left to be indicated).
For your concern, I agree to leave Subject input there with MMS indicator.
Assignee | ||
Updated•11 years ago
|
Attachment #8403473 -
Flags: review- → review?(borja.bugzilla)
Updated•11 years ago
|
Target Milestone: 2.0 S1 (9may) → 2.0 S2 (23may)
Assignee | ||
Comment 49•11 years ago
|
||
Attachment #8419381 -
Flags: ui-review?(vpg)
Assignee | ||
Comment 50•11 years ago
|
||
Attachment #8419382 -
Flags: ui-review?(vpg)
Assignee | ||
Comment 51•11 years ago
|
||
Attachment #8419383 -
Flags: ui-review?(vpg)
Assignee | ||
Comment 52•11 years ago
|
||
Attachment #8419384 -
Flags: ui-review?(vpg)
Updated•11 years ago
|
Attachment #8419381 -
Flags: ui-review?(vpg) → ui-review+
Updated•11 years ago
|
Attachment #8419382 -
Flags: ui-review?(vpg) → ui-review+
Updated•11 years ago
|
Attachment #8419383 -
Flags: ui-review?(vpg) → ui-review+
Updated•11 years ago
|
Attachment #8419384 -
Flags: ui-review?(vpg) → ui-review+
Reporter | ||
Comment 53•11 years ago
|
||
Comment on attachment 8403473 [details] [review]
Path in github
Re-enabling the flag to r? to Julien!
Attachment #8403473 -
Flags: review- → review?(felash)
Updated•11 years ago
|
feature-b2g: --- → 2.0
Comment 54•11 years ago
|
||
(In reply to Omega Feng [:Omega] from comment #48)
> (In reply to Victoria Gerchinhoren [:vicky] from comment #44)
> > (In reply to Omega Feng [:Omega] from comment #43)
> > Yes, I agree that subject is not that important, but for me the important
> > thing is to show the user how adding subject to a regular SMS will turn it
> > into a MMS being a much more expensive type of message in our target
> > countries. So the idea of the MMS indicator to be next to the subject when
> > there is so, is just to somehow advice -with a visual connection of
> > elements- that one of the reasons of being an MMS is that Subject addition
> > (a user already knows that adding a file will cost more, the second
> > situation of having a really long message turning into an MMS is already
> > communicated by an in app message and this third way to convert the message
> > into an MMS is the one left to be indicated).
>
> For your concern, I agree to leave Subject input there with MMS indicator.
Then we'll need to handle this in a separate bug, thanks.
Joan, can you please file a new bug for the subject handling, as we discussed in comment 39?
Comment 55•11 years ago
|
||
> Joan, can you please file a new bug for the subject handling, as we
> discussed in comment 39?
done!, please refer to Bug 1008127
Reporter | ||
Comment 56•11 years ago
|
||
Comment on attachment 8403473 [details] [review]
Path in github
Some comments in Github! Take a look and let me know when ready! Great job Joan! :)
Attachment #8403473 -
Flags: review?(borja.bugzilla) → feedback+
Comment 57•11 years ago
|
||
Comment on attachment 8403473 [details] [review]
Path in github
This is going well but there is still some work to do.
Added some comments on github.
See also Victoria's answer in in bug 950175 comment 59 (which is the behavior that this patch shows, so it's already perfect ;) ).
Attachment #8403473 -
Flags: review?(felash)
Assignee | ||
Updated•11 years ago
|
Attachment #8403473 -
Flags: feedback+ → review?(borja.bugzilla)
Comment 58•11 years ago
|
||
Why do you ask the review again but you didn't fix the various comments?
(In reply to Julien Wajsberg [:julienw] from comment #58)
> Why do you ask the review again but you didn't fix the various comments?
Julien, I guess Joan has fixed what it is in his hands. As commented in github he is asking any of you for help on the javascript side.
This is a huge bug, and it would be great if someone in your team could help him land it.
Comment 60•11 years ago
|
||
That's not true: a lot of the comments Borja and I made are in the CSS. Only a small part is in the JS.
There are also some questions where we just need answers because, well, we're not in Joan's head and sometimes we just don't understand why some things are made this way.
Also, I think the JS side should really be removed and transformed to a CSS-only solution. Especially, I'd like to know what part of the design the JS changes are trying to solve, and then we can either try to solve the issue or discuss with Vicky to find another closer design.
Honestly, I think Joan and you have more skills than me to answer these questions, but someone from our team can definitely help.
Hope this helps!
(In reply to Julien Wajsberg [:julienw] from comment #60)
> That's not true: a lot of the comments Borja and I made are in the CSS. Only
> a small part is in the JS.
Sorry for that I just went through the comments on github and did not verified the PR updates.
I trusted Joan would have fixed what was in his hands.
> There are also some questions where we just need answers because, well,
> we're not in Joan's head and sometimes we just don't understand why some
> things are made this way.
>
> Also, I think the JS side should really be removed and transformed to a
> CSS-only solution. Especially, I'd like to know what part of the design the
> JS changes are trying to solve, and then we can either try to solve the
> issue or discuss with Vicky to find another closer design.
>
> Honestly, I think Joan and you have more skills than me to answer these
> questions, but someone from our team can definitely help.
I have not been on top of all changes as I'm refreshing all apps but SMS and dialer.
But I'll try to work closer with Joan to get more involved in SMS.
> Hope this helps!
Assignee | ||
Comment 62•11 years ago
|
||
Hi,
I update de PR with css changes, I add comment about JS https://github.com/mozilla-b2g/gaia/pull/18088#discussion_r12611472
Flags: needinfo?(felash)
Comment 63•11 years ago
|
||
There are still a lot of comments that are not addressed but let's focus on the JS issue for now.
Oleg, would you have an idea on how to support these mockups: attachment 8394786 [details] and attachment 8394787 [details] using CSS only? A difficult part is to be able to make the composer height stop at the bottom of the recipients panel (but maybe we can cheat and always make it stop at the bottom of the bigger-style panel)
Flags: needinfo?(felash) → needinfo?(azasypkin)
Comment 64•11 years ago
|
||
(In reply to Julien Wajsberg [:julienw] from comment #63)
> There are still a lot of comments that are not addressed but let's focus on
> the JS issue for now.
>
> Oleg, would you have an idea on how to support these mockups: attachment
> 8394786 [details] and attachment 8394787 [details] using CSS only? A
> difficult part is to be able to make the composer height stop at the bottom
> of the recipients panel (but maybe we can cheat and always make it stop at
> the bottom of the bigger-style panel)
Looking at mockups I'm wondering why can't we use flex-boxes here, where top (recipients) and bottom(input) blocks can grow and middle block will occupy the rest of the space? I don't know this particular markup well though, maybe it's difficult to change.
Flags: needinfo?(azasypkin)
Assignee | ||
Comment 65•11 years ago
|
||
(In reply to Oleg Zasypkin [:azasypkin] from comment #64)
> (In reply to Julien Wajsberg [:julienw] from comment #63)
> > There are still a lot of comments that are not addressed but let's focus on
> > the JS issue for now.
> >
> > Oleg, would you have an idea on how to support these mockups: attachment
> > 8394786 [details] and attachment 8394787 [details] using CSS only? A
> > difficult part is to be able to make the composer height stop at the bottom
> > of the recipients panel (but maybe we can cheat and always make it stop at
> > the bottom of the bigger-style panel)
>
> Looking at mockups I'm wondering why can't we use flex-boxes here, where top
> (recipients) and bottom(input) blocks can grow and middle block will occupy
> the rest of the space? I don't know this particular markup well though,
> maybe it's difficult to change.
If we use flex-box for the list of recipients, the behavior of the pills does not behave as desired.
Assignee | ||
Comment 66•11 years ago
|
||
Several comments: https://github.com/mozilla-b2g/gaia/pull/18088
Flags: needinfo?(felash)
Comment 67•11 years ago
|
||
Just to note that I gave a lot of thought to this between yesterday and today (yeah, while I was trying to sleep ;) ) and I think we should leave the JS part out of this patch and move it to another bug (like we did for the subject handling).
So that we can move forward with the bigger changes, and maybe rediscuss with Victoria for this specific issue. I couldn't find a CSS only solution, but I'll outline the requirements we have for this panel on the other bug so maybe someone will be able to help.
Would that work for you Joan?
Flags: needinfo?(felash)
Assignee | ||
Comment 68•11 years ago
|
||
Okay for me, I think you'd better split, because this bug is becoming a meta.
Comment 69•11 years ago
|
||
So Joan, can you update your current patch with removing all things related to the recipient height handling between 1 line and more than 1 line? I think we can keep the current behavior for now, using the 1-line-and-a-half display, even if we have only 1 line of recipients.
Assignee | ||
Comment 70•11 years ago
|
||
Ok, working on it and I update the PR
Assignee | ||
Comment 71•11 years ago
|
||
Ok, working on it and I update the PR
Assignee | ||
Comment 72•11 years ago
|
||
I updated the Patch, now the recipients container only shows one line.
https://github.com/mozilla-b2g/gaia/pull/18088
Flags: needinfo?(felash)
Assignee | ||
Updated•11 years ago
|
Attachment #8403473 -
Flags: review?(borja.bugzilla) → review?(felash)
Assignee | ||
Updated•11 years ago
|
Flags: needinfo?(felash)
Comment 73•11 years ago
|
||
I haven't finished completely the review but still added some comments on github.
Also, there is an issue when the send button is bigger (for example in french) but that should be fine when we switch to an icon, so let's not take care of this.
Comment 74•11 years ago
|
||
Victoria, I just think we miss an important piece of information: how a currently-edited recipient looks like.
Historically it used to look like an "assimilated" recipient, only without the "recipient pill". Should it be the same with the VR?
Comment 76•11 years ago
|
||
Comment on attachment 8403473 [details] [review]
Path in github
So, this looks good.
now I'd like just some comments in the CSS, and I need Victoria answer for the currently edited recipients.
Comment 77•11 years ago
|
||
(In reply to Julien Wajsberg [:julienw] from comment #74)
> Victoria, I just think we miss an important piece of information: how a
> currently-edited recipient looks like.
>
> Historically it used to look like an "assimilated" recipient, only without
> the "recipient pill". Should it be the same with the VR?
Sure, this didn't change. I have already checked that behaviour wuth Joan.
Thanks for bringing it up in any case ;)
Flags: needinfo?(vpg)
Comment 78•11 years ago
|
||
Victoria, just to be clear ;)
* should the "currently-edited" recipient have the same style as the "assimilated" recipient (italic, font-size), or should it have the same style than before the refresh (not italic, bigger font-size)?
It's currently the latter.
Flags: needinfo?(vpg)
Updated•11 years ago
|
Target Milestone: 2.0 S2 (23may) → 2.0 S3 (6june)
Comment 79•11 years ago
|
||
(In reply to Julien Wajsberg [:julienw] from comment #78)
> Victoria, just to be clear ;)
>
> * should the "currently-edited" recipient have the same style as the
> "assimilated" recipient (italic, font-size), or should it have the same
> style than before the refresh (not italic, bigger font-size)?
>
> It's currently the latter.
Same before the refresh, so both states are really different.
THanks!
Flags: needinfo?(vpg)
Comment 80•11 years ago
|
||
Comment on attachment 8403473 [details] [review]
Path in github
r=me, let's land this
I can fix the comments myself but I left some questions about the "white-space: pre-wrap;" changes. Arnau, do you know the answer?
Attachment #8403473 -
Flags: review?(felash) → review+
Flags: needinfo?(arnau)
Comment 81•11 years ago
|
||
Joan, I don't know if you have the time to answer this (see comment 80)
Flags: needinfo?(joan.leon)
(In reply to Julien Wajsberg [:julienw] from comment #80)
> Comment on attachment 8403473 [details] [review]
> Path in github
>
> r=me, let's land this
>
> I can fix the comments myself but I left some questions about the
> "white-space: pre-wrap;" changes. Arnau, do you know the answer?
No idea, Joan will have to answer.
I've installed the patch and looks like the option pre-wrap has no effect, maybe it was used to override a previous value, but FWIW white-space could be removed.
Flags: needinfo?(arnau)
Assignee | ||
Comment 83•11 years ago
|
||
Julien, appears to be a residue of a test, as it can remove said Arnau ...
Flags: needinfo?(joan.leon)
Assignee | ||
Comment 84•11 years ago
|
||
I updated PR https://github.com/mozilla-b2g/gaia/pull/18088
\o/ Good job guys!
Comment 87•11 years ago
|
||
I've noticed a localization issue: if "To" is made bigger because of localization, the panel will override it. I'm filing a follow-up for this.
Comment 88•11 years ago
|
||
Filed bug 1017018.
Comment 89•11 years ago
|
||
master: 4142df90c71abdc1e3a87cd37dff1a22d5e38b34
I also found bug 1017024 while testing, but let's handle this as a bug.
Status: NEW → RESOLVED
Closed: 11 years ago
Flags: needinfo?(felash)
Resolution: --- → FIXED
Updated•11 years ago
|
status-b2g-v2.0:
--- → fixed
Comment 90•11 years ago
|
||
Tested and fine
Hamachi
2.0
Gecko-c5087e3
Gaia-5dfe4ec
Status: RESOLVED → VERIFIED
Updated•11 years ago
|
Updated•10 years ago
|
blocking-b2g: backlog → ---
tracking-b2g:
--- → backlog
You need to log in
before you can comment on or make changes to this bug.
Description
•