Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Bug 809974: Display "Last Updated" date and time in Device Info. r=kaze #6560

Merged
merged 5 commits into from

5 participants

@marshall
Collaborator
apps/settings/style/settings.css
@@ -227,6 +227,12 @@ ul li > label .range-icons.brightness {
pointer-events: none;
}
+/******************************************************************************
+ * Device Information
+ */
+#last-update-date {
+ font-size: 1.4rem;
+}
@fabi1cazenave Collaborator

That’s my only nit: why do we need a specific rule for this item? I have to ask, as we’re trying to improve the UI consistency in the Settings app…

@marshall Collaborator

the date and time will overlap the "Last updated" label otherwise. I hunted around for a classname I could reuse but didn't see any -- feel free to enlighten me :). I also found a bug if no Last Updated date was found, so I will push that fix to my branch soon.

Do we have any automated way to test the settings UI code?

@fabi1cazenave Collaborator

In this case, I’d suggest to put the date below the label instead of next to it — think of other locales for which the text will be even trickier to fit.

<li>
  <small>2012-11-24</small>
  <a> Last updated </a>
</li>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@marshall
Collaborator

@jcarpenter Do you have a preference for the text of Device Info > Last Updated when there wasn't a last updated date / time? Some possibilities:

"No update applied yet"
"Never"
.. or just leave it empty ?

@jcarpenter
Collaborator

@marshall

Do you have a preference for the text of Device Info > Last Updated when there wasn't a last updated date / time?

What about the device install date? Or empty is also fine. Easier for l10n team.

@jcarpenter
Collaborator

@fabi1cazenave

In this case, I’d suggest to put the date below the label instead of next to it — think of other locales for which the text will be even trickier to fit.

Let's keep it on the right. Making an exception for this one field would break the consistency of the left-right layout. In the relatively unlikely event that the date exceeds the available space, we can wrap the text.

@fabi1cazenave
Collaborator

@jcarpenter it is very likely that the targeted locales will require more space than English to display the translation of “last update”, and there are two problems with this approach:

  • for these items where the heading and the data are on the same line, we currently have no way to wrap the heading automatically (that’s why we only use this when we’re sure it won’t overlap);
  • wrapping the heading text would not help for Spanish or Portuguese: for these latin languages the important word (“update”) will be the last one, thus dropped if the text is wrapped.

Of course, there are other panels where the data is below the heading (e.g. main panel). And for the “Software” item above this “Last Updated” one, @lco has already suggested to use two lines instead of one: https://bugzilla.mozilla.org/show_bug.cgi?id=808892

However, if the priority is to make this “Device information” panel look good in English then we need a better way to divide the space between the heading and the data — e.g. use a fixed width of 50% for heading and data, and wrap the text accordingly. As you can guess, I’d still find this silly.

@marshall
Collaborator

@jcarpenter @fabi1cazenave Here's a screenshot of what it currently looks like, if it helps
last updated

@fabi1cazenave
Collaborator

@marshall the date string size is stable and predictable, but the heading string size is very dependent on the locale. Your screenshot looks nice for English but I’d much, much prefer to put this on two lines to ensure it’ll still be readable for all locales.

@marshall
Collaborator

@fabi1cazenave @jcarpenter new screenshot
last updated

@fabi1cazenave
Collaborator

this is fine to me

@fabi1cazenave
Collaborator

@marshall would you please attach this PR to bug 809974 so I can r+ it and merge it?

@jcarpenter
Collaborator

for these items where the heading and the data are on the same line, we currently have no way to wrap the heading automatically (that’s why we only use this when we’re sure it won’t overlap);
wrapping the heading text would not help for Spanish or Portuguese: for these latin languages the important word (“update”) will be the last one, thus dropped if the text is wrapped.
Of course, there are other panels where the data is below the heading (e.g. main panel). And for the “Software” item above this “Last Updated” one, @lco has already suggested to use two lines instead of one: https://bugzilla.mozilla.org/show_bug.cgi?id=808892

Suggested, but it has not been agreed upon internally by UX, and it was not planned for v1.

Long term we need a better solution to overruns and collisions. Other mobile OS avoid this by:

  • Being smart about localization
  • Automatically tightening character spacing
  • Automatically adding elipsis
  • Wrapping text lines

What we cannot do is hard code in exceptions case by case. We're going to miss some, inevitably. And it will look terrible if some fields are right aligned, and some are spread across two lines. Consistency matters.

I can live with this exception for v1, but in future please wait for UX approval before requesting merge, especially when my last comment explicitly asked for the opposite approach.

@fabi1cazenave
Collaborator

then let's go with the smaller font-size then.

@fabi1cazenave
Collaborator

@marshall please still open a bug and ask r? from somebody else. Thanks!

@vingtetun
Collaborator

@jcarpenter

  • Being smart about localization I don't have the feeling that the default design is making English strings that will translate shortly in other languages. Localizers spend a lot of time trying to keep the initial meaning of the string and to fit in the UI. Sadly this is hard for some languages that use strings longer than english, you should try b2g in french and try to navigate a bit in the settings...

So the first smart localization is on the original strings (aka original design), they need to be thinked so the translation will not be longer.

  • Tightening character spacing
    That would be nice to have but CSS does not have a magic property that does that if the fields does not fit in the container. You may want to add a proposal for letter-spacing: automatic. Doing that on a per-app basis is a fail too, it needs to be a standard fix.

  • Automatically adding ellipsis
    This is not a proof of being smart. It makes me upset when a screen contains ellipsis for default values. Is the phone not supposed to be designed to be usable in my language?

  • Wrapping text line
    This one sounds good too for me.

Anyway I don't consider that we should block on UX issue. It can be fixed into a different bug that is not blocking )or you can ask blocking on it if you want) but we need to resolve the functional issues first, even if the UI is not perfect yet.

@fabi1cazenave
Collaborator

Just realized that I understood “ellipsed” instead of “wrapped”, sorry for the confusion. I agree wrapping text would be far more acceptable that truncating it, but it would make the list item bigger (which you might not like).

Other mobile OS avoid this by […]

Other mobile OS have their own graphical widgets, with their pros and cons; at this stage of the project I’d much prefer to first implement what the platform can do easily.

@marshall
Collaborator

@fabi1cazenave I've attached the PR to Bug 809974 and marked you for r?

Just to be clear -- do I need to revert back to "on the same line" behavior in this pull request?

@evelynhung
Collaborator

@marshall
the patch can't be automatically merged probably because it conflicts with my latest commit of settings.js or index.html.
Could you please update to the latest Gaia and resolve the conflicts carefully? Thanks!

@fabi1cazenave
Collaborator

Just to be clear -- do I need to revert back to "on the same line" behavior in this pull request?

@marshall as my opinion regarding localization issues in the Settings app is not relevant, I’ll let you see that with UX. And since @evelynhung jumped in, I think it qualifies her for a review. :-)

Thanks for your work, sorry for the confusion here.

@evelynhung
Collaborator

Hi @marshall, in my PR #6771 I've made all fields in two lines, so I think your PR is fine, just need to resolve all conflicts so we can merge it.

@jcarpenter If the two-line display is not good for UX, please file another issue, we can address the problem there. Thanks!

@jcarpenter
Collaborator

@vingtetun

. . . we need to resolve the functional issues first, even if the UI is not perfect yet.

Agreed. I can live with the two lines approach for v1, if the code is already written and merged. We have bigger things to deal with.

Everything else in your response I agree with.

  • Smart localization is our best tool.
  • In the future, our English-speaking UX team should allow for extra space in our layouts, in anticipation of the longer strings of other languages.
  • In the future we can look into adding automatic adaptations, eg: elipsis, compressed letter spacing, and line wraps.
@marshall marshall Merge branch 'master' of git://github.com/mozilla-b2g/gaia into lastU…
…pdated

Conflicts:
	apps/settings/index.html
	apps/settings/style/settings.css
e78b094
@marshall
Collaborator

@evelynhung I've merged and tested and all looks good

@marshall
Collaborator

@evelynhung is this good to merge?

@fabi1cazenave
Collaborator

Merging, as @evelynhung has already r+’ed your patch.

@fabi1cazenave fabi1cazenave merged commit 46baef6 into from
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Nov 21, 2012
  1. @marshall
Commits on Nov 27, 2012
  1. @marshall
Commits on Nov 28, 2012
  1. @marshall
Commits on Nov 29, 2012
  1. @marshall
Commits on Dec 4, 2012
  1. @marshall

    Merge branch 'master' of git://github.com/mozilla-b2g/gaia into lastU…

    marshall authored
    …pdated
    
    Conflicts:
    	apps/settings/index.html
    	apps/settings/style/settings.css
This page is out of date. Refresh to see the latest.
Showing with 26 additions and 2 deletions.
  1. +2 −2 apps/settings/index.html
  2. +24 −0 apps/settings/js/settings.js
View
4 apps/settings/index.html
@@ -1727,8 +1727,8 @@ <h1 data-l10n-id="deviceInfo"> Device Information </h1>
<a id="os-version" data-l10n-id="software"> Software </a>
</li>
<li>
- <small data-name="deviceinfo.last_updated"></small>
- <a id="last-update-date" data-l10n-id="last-updated"> Last Updated </a>
+ <small id="last-update-date"></small>
+ <a data-l10n-id="last-updated"> Last Updated </a>
</li>
<li>
<label>
View
24 apps/settings/js/settings.js
@@ -339,6 +339,29 @@ var Settings = {
req.send();
},
+ loadLastUpdated: function settings_loadLastUpdated() {
+ var settings = this.mozSettings;
+ if (!settings) {
+ return;
+ }
+
+ var lastUpdateDate = document.getElementById('last-update-date');
+ var lock = settings.createLock();
+ var key = 'deviceinfo.last_updated';
+ var request = lock.get(key);
+ request.onsuccess = function() {
+ var lastUpdated = request.result[key];
+ if (!lastUpdated) {
+ return;
+ }
+
+ var f = new navigator.mozL10n.DateTimeFormat();
+ var _ = navigator.mozL10n.get;
+ lastUpdateDate.textContent = f.localeFormat(new Date(lastUpdated),
+ _('shortDateTimeFormat'));
+ };
+ },
+
openDialog: function settings_openDialog(dialogID) {
var settings = this.mozSettings;
var dialog = document.getElementById(dialogID);
@@ -627,6 +650,7 @@ window.addEventListener('load', function loadSettings() {
document.getElementById('ftuLauncher').onclick =
Settings.launchFTU.bind(Settings);
Settings.loadGaiaCommit();
+ Settings.loadLastUpdated();
break;
case 'help': // handle specific link
Settings.getUserGuide(function userGuideCallback(url) {
Something went wrong with that request. Please try again.