951080 - [Clock] Adjustment for setting a new or editing an alarm

Status: VERIFIED FIXED

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

Description

[Helen Huang [:HHuang]]

Remove text-shadow from dropdown buttons, and font-size adjustment.

Comment 1

[Gnaneswar]

If possible, could you attach screen shot explaining the issue?

Comment 2

[Helen Huang [:HHuang]]

Attachment: Alarm setting screenshot.jpg

I've attached the screenshot and noted on it, please take a look.

Comment 3

[Gnaneswar]

Attachment: 0001-Bug-951080-Clock-Adjustment-for-setting-a-new-or-edi.patch

Thanks for the spec.

Attached patch fix First part of the issue(i.e setting font-size, family and weight for all labels).

Second part:
To remove the text-shadow, I tried using  following way in alarm.css but somehow this seems have no effect on text-shadow.

Ex: To remove text-shadow for Repeat

#repeat-select-menu {
  text-shadow: 0 0;
}

Please let me know your comments.

Comment 4

[Helen Huang [:HHuang]]

Sorry for the late reply.
If there is no effect on text-shadow, it probably has been removed following common controls… That will be fine.
But, I still need a screenshot to review, I can not install patch on my device.
Thank you!

Comment 5

[Gnaneswar]

Attachment: Screenshot

Attached screen shot taken which includes 
 a) Changes to fonts for labels as specified in spec
 b) Removing text shadow and setting the fonts to drop-down selection items

BTW,As per the spec, font-weight "medium" to be set for drop-down selection items.

It seems we can set font-weight to only one of these(normal OR bold OR bolder OR lighter)
Could you share the details of which one of these corresponding to medium?

Currently in the attached screen shot I set it as bold. Please confirm it.

Comment 6

[Gnaneswar]

Attachment: 0001-Bug-951080-Clock-Adjustment-for-setting-a-new-or-edi.patch

Attached screen shot's corresponding patch for reference.

>>Currently in the attached screen shot I set it as bold. Please confirm it.
Sorry,typed wrong earlier.set it as "normal"

Comment 7

[Gnaneswar]

Would like to know your comments as well.

Comment 8

[Marcus Cavanaugh [:mcav]]

Appearance looks OK to me if that's what Helen wants; the patch itself should condense the CSS selectors into one block, rather than a separate block for each ID.

Comment 9

[Gnaneswar]

Attachment: Pointer to Github PR

Thanks Marcus for comments.Updated attached patch as per comments.

Helen,
Requesting you to review the patch.

Comment 10

[Helen Huang [:HHuang]]

Attachment: Alarm setting screenshot_Jan27.jpg

Following font-weight we have, I’ve modified the spec. Please set font-weight to drop-down selection items as normal, and item title is normal, too. I also modified the font size. 

Otherwise, I checked the attached you provided, since shadow effect on drop-down items still exist… maybe it inherits the style from somewhere. Could you help to check again?

Thank you!

Comment 11

[Gnaneswar]

So far I couldn't get clue as to why text-shadow properties are not being effected.

Comment 12

[Marcus Cavanaugh [:mcav]]

Attachment: Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/15921

Comment 13

[Marcus Cavanaugh [:mcav]]

Attachment: Screenshot

Helen, please take a look at this attachment and let me know if this matches the spec you're looking for.

Gnaneswar, you may find it helpful when debugging issues like these to use the Firefox Web Inspector. Its sidebar shows detailed CSS info, such as where styles are being inherited from, whether or not they've been overridden, and other stuff like that. Also, in general, it's best to try to keep the CSS as concise as possible -- i.e. if there are styles that already exist for the elements in question, try to piggyback on those, and omit styles that are already cascading to what you want. Take a look at the patch I've provided and feel free to ask me if you have any questions, I'm happy to explain if you have more specific questions about why something works or doesn't work. :-)

Comment 14

[Gnaneswar]

Marcus,
Thanks for the information. I am exploring firefox web inspector.

BTW, I have one comment w.r.t patch.

In timer.css (see code snippet below) text-shadow is being set to zero. I guess after your patch, this statement may not be required. please confirm this.

#timer-sound-menu {
  border-width: 0;

  color: #FFF;
  text-align: left;
  background-color: #4D4D4D;
  text-shadow: 0 0;
}

Comment 15

[Marcus Cavanaugh [:mcav]]

Yes, that seems correct.

Comment 16

[Helen Huang [:HHuang]]

Comment on attachment 8369583 [details]
Screenshot

Marcus, thanks for helping :) Font-size and text effect is correct.

Just want to make sure are the font-weight for item's title and drop-down selection items set "Normal" already? They look a bit strong.

Besides, for the third item "sound"... the top of the letter "S" is cropped, maybe it caused by text field, can you check it as well?

Comment 17

[Marcus Cavanaugh [:mcav]]

Attachment: screen.png

Ah, I had taken that screenshot from Firefox Nightly, which I forgot renders fonts differently because it doesn't have Fira Sans. Take a look at this screenshot, which I took from on-device, which I think shows that all of the things you mentioned were just glitches from being rendered in Nightly.

Comment 18

[Gnaneswar]

(In reply to Marcus Cavanaugh [:mcav] <[e-mail]> from comment #15)
> Yes, that seems correct.

So, would like to know if it will removed as a part of your patch?

Comment 19

[Marcus Cavanaugh [:mcav]]

I'll verify that it actually is unnecessary, and then yeah, I'll remove that too.

Comment 20

[Helen Huang [:HHuang]]

Comment on attachment 8370682 [details]
screen.png

Marcus, it looks great! Thank you for clarifying this.

Comment 21

[Marcus Cavanaugh [:mcav]]

Landed in master:

https://github.com/mozilla-b2g/gaia/commit/7c1a4ce0b6ab7e3fb51e0c015a06e5255b2be58b

Comment 22

[Edward Chen[:edchen]]

[Verified info]
Gaia      6e71ab4da1b08586ea0c758edb7aa199ee34cd2f
Gecko     https://hg.mozilla.org/mozilla-central/rev/bb030d47c946
BuildID   20140219160202
Version   30.0a1

[Comment]
Appearance looks OK for me, it has not UI issue occurred. I mark it to VERIFIED.

Comment 23

[Helen Huang [:HHuang]]

Comment on attachment 8364980 [details] [review]
Pointer to Github PR

Bug fixed