[Dialer] Replace span with hidden mechanism in CallScreen.js in showClock

RESOLVED FIXED

Status

Firefox OS
Gaia::Dialer
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: thills, Assigned: gsvelto)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [dialer-most-wanted])

Attachments

(1 attachment)

(Reporter)

Description

3 years ago
In the showClock function, a <span> is used to hide the AM/PM from the time that shows up when a user receives an incoming call.  We should use a hidden mechanism here.
After thinking about this some more, I don't think we really need to do anything here other than comment what's happening better. Adding a .hidden class would be disingenuous anyways since the span is shown in the mozHour12=true case. Thanks for filing this anyways.
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Depends on: 1035763
Resolution: --- → WONTFIX
Reopened as I was wrong about the visibility of this element depending on the mozHour12 setting.

This also lacks decent test coverage, so we should fix that too. There's one test that covers it indirectly.
Status: RESOLVED → REOPENED
Resolution: WONTFIX → ---
Whiteboard: [dialer-most-wanted]
(Reporter)

Updated

3 years ago
Summary: [Dialer] Remove span with hidden mechanism in CallScreen.js in showClock → [Dialer] Replace span with hidden mechanism in CallScreen.js in showClock
(Assignee)

Updated

3 years ago
Assignee: nobody → gsvelto
Status: REOPENED → ASSIGNED
(Assignee)

Comment 3

3 years ago
Created attachment 8625194 [details] [review]
[PULL REQUEST] Remove the use of innerHTML for displaying the time in the lockscreen r=drs,gweng

This removes the use of the hidden span to hide the AM/PM component of the clock. Instead it strips the unwanted part of the string via a regular expression. I've also removed the now redundant CSS selector. I've done the same change in the lockscreen since the code is essentially the same hence the double review (Doug for the callscreen and Greg for the lockscreen).

Note that I've written the patch on top of bug 1176193 and since that's not merged yet the patches appear in the PR but there's no interactions between the two save for the eslint xfail.list.
Attachment #8625194 - Flags: review?(gweng)
Attachment #8625194 - Flags: review?(drs)
Comment on attachment 8625194 [details] [review]
[PULL REQUEST] Remove the use of innerHTML for displaying the time in the lockscreen r=drs,gweng

I think somethings need to be elaborated more, so I left some comments on the PR.  Thanks.
Attachment #8625194 - Flags: review?(gweng)
(Assignee)

Comment 5

3 years ago
Comment on attachment 8625194 [details] [review]
[PULL REQUEST] Remove the use of innerHTML for displaying the time in the lockscreen r=drs,gweng

I've updated the comment regarding the AM/PM component of the clock on the lockscreen. Hopefully this should make the intent clearer.
Attachment #8625194 - Flags: review?(gweng)
Comment on attachment 8625194 [details] [review]
[PULL REQUEST] Remove the use of innerHTML for displaying the time in the lockscreen r=drs,gweng

The part now looks good. Thanks!
Attachment #8625194 - Flags: review?(gweng) → review+
Comment on attachment 8625194 [details] [review]
[PULL REQUEST] Remove the use of innerHTML for displaying the time in the lockscreen r=drs,gweng

It would be nice if we could think of a better method to strip away the AM/PM tag, but I can't.
Attachment #8625194 - Flags: review?(drs) → review+
(Assignee)

Comment 8

3 years ago
I think this is one of the use cases we should be able to clean up with the new l10n libraries, the date support isn't there yet though AFAIK.
(Assignee)

Comment 9

3 years ago
Merged to gaia/master 8c65fa5e3879ecb528206494998ce0e58c1f77a9

https://github.com/mozilla-b2g/gaia/commit/8c65fa5e3879ecb528206494998ce0e58c1f77a9
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago3 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.