803238 - Week view: If the text to display by default in the title (e.g. dow, month, year) exceeds the view length, use short names for day of week, month, year, etc

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::Calendar, defect, P3)

Description

[Jason Smith [:jsmith]]

Build:

Device: Otoro
Hashes:

  <project name="gaia" path="gaia" remote="b2g" revision="e3efbd0411218762cf9a62278bf58ee513ff331f"/>
  <project name="releases-mozilla-aurora" path="gecko" remote="mozilla" revision="38c06c8b4f8f8a6a513a66ab62c9421835017c9f"/>

Steps:

1. Go to a week with November and December crossing over in the same week view (example: November and December 2012)

Expected:

I'd expect that if we run out of space on screen, that we would do short-hand naming for the months rather than the full name to be able to get the core information on screen (what months and what year(s)).

Actual:

...mber December 2012 is seen as the title. Definitely not blocker worthy, but we could polish this up a bit.

Comment 1

[James Lal [:lightsofapollo]]

I like the approach of using the short month names when we need to display two months.
This might also make enough room to display both years when in December.

Comment 2

[Jason Smith [:jsmith]]

Oh btw - I should note another case this could happen is that long day of the week + month + day. I should probably generalize this to "if you run out of the text on the title, use short-names instead for days of week, months, etc"

Comment 4

[Eike Send]

I have created two versions that fix this problem. 

The first one actually measures the width of the text and uses a shorter date notation if it becomes too wide. This adresses the case mentioned comment 2 as well. This patch can be found here, it is a bit longer because of the function for measuring the width:

https://github.com/eikes/gaia/commit/14e43fd9b7b50e475770126f23b04002bb9d4ed3

The second version takes the simpler route of always shortening the month names if two months are involved in a week view. This patch can be found here:

https://github.com/eikes/gaia/commit/a601b509f236c9689da1fe3fb557e5c690e0dc55

If you want me to, I can attach the diffs as patches, but maybe that's not necessary, I'm not sure what the proper etiquette is. Maybe you can let me know which one you prefer and I'll attach it here or open a pull request on github or both.

Comment 5

[Casey Yee]

(In reply to James Lal [:lightsofapollo] from comment #1)
> I like the approach of using the short month names when we need to display
> two months.
> This might also make enough room to display both years when in December.

+1
I think this makes sense.

Comment 6

[James Lal [:lightsofapollo]]

I think this should block, we needed to remove my band-aid that let us actually see the current month by using

Comment 7

[James Lal [:lightsofapollo]]

... a quick hack (direction: rtl) to cut off the previous month. There is a platform bug that causes this to permanently truncate text so we needed to remove it otherwise the entire title would disappear.

Now we have a similar situation where its possible the current month becomes unreadable because of the size of text. In English this is fairly bad but I suspect its much worse in other locales. We already have a mostly complete patch for this (eikes++) but we should bring it to completion.

Comment 8

[Eike Send]

I just  wanted to say, that I'm still willing to improve and document my code. Just let me know what the requirements are and I think I'll be able to create another patch fairly quickly. 

If you want the title.clientWidth cached, I'll do it, it's easy enough, even though I doubt it'll make any difference performance wise.

Comment 9

[Jason Smith [:jsmith]]

Personally, I wouldn't block on this given the level of severity we upped the blocking call too, but I'd track it.

It's certainly a usability issue that could happen, but I don't think I'd hold the release on it.

Comment 10

[James Lal [:lightsofapollo]]

:) Great,

Lets skip the caching for now... Its fairly easy to see the performance difference (if any). The only thing that blocks this is some unit tests (https://github.com/mozilla-b2g/gaia/#unit-tests). The text should be English by default in all tests which should make this fairly easy I think.

I will assign you. Please reach out to me if you need help.

Comment 11

[Eike Send]

I just added two unit tests to the pull request. Please let me know if they are sufficient, I found them to be meaningful enough.

https://github.com/mozilla-b2g/gaia/pull/6851/commits

Comment 12

[u466883]

Attachment: not shortened month names

Comment 13

[Sarah Parsons]

Attachment: Dec-Jan Overlap 4-23-13

Comment 14

[Sarah Parsons]

This issue is worse on the Leo device. Overlapping of the + button will occur instead of ellipses. Updated screenshot has been attached. 

Leo Build ID: 20130411070205
Environmental  Variables:
Kernel Date: Mar 15
Gecko: http://hg.mozilla.org/releases/mozilla-b2g18/rev/f671fa539473
Gaia: e7e338a765e22334b40ced41489a785941382c66
UCID:calendar-038

Comment 15

[Josh Matthews [:jdm]]

James, did Eike's update in January receive any feedback?

Comment 16

[James Lal [:lightsofapollo]]

I dropped the ball here... The patch no longer applies but the logic is sound and I can rework it to apply. Reassigning to myself for cleanup.

Comment 17

[James Lal [:lightsofapollo]]

Attachment: Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/9830

Pointer to Github pull-request

Comment 18

[Kevin Grandon :kgrandon]

Comment on attachment 750844 [details]
Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/9830

Sorry for the delay. I'm concerned about the heavy javascript performance impact of getTextWidth() in this patch.

I would prefer a CSS based approach, or ideally only showing a single month in the header.

Comment 19

[James Lal [:lightsofapollo]]

my goal here was to clean up the existing patch with the r- we need to rethink our approach so I am going to unassign myself from this for the moment...

Comment 20

[J Zimbrick]

This issue seems to affect all 1.2 shipping locales.

I'm seeing the same issue occurring in  Catalan/Català, Czech/Čeština, Greek/ελληνικά, Croatian/Hrvatski, Hungarian/Magyar, Italian/Italiano, Dutch/Nederlands, Polish/Polski, Russian/Русский, and Slovak/Slovenčina.

Also, truncation can appear in the day view if the month/day/date combo is particularly long.

Will the fix being discussed solve the issue for all languages? Or is this an issue for the localisers or UI teams? And if so, should seperate issues be filed for each language?

This was found on the latest Buri 1.2 Com Build:

Gaia   1fd315337d8ae891c3024e4c682c4c50797ea40e
SourceStamp d585fe28cd55
BuildID 20131021004006
Version 26.0a2

Comment 21

[Miller Medeiros [:millermedeiros]]

Attachment: pull request #2

I took a simpler approach for the date handling which I think will have less performance impact. By using the short form of the months ("Dec", "Jan"...) we can also fit the year of both months - which works better for the edge case of "Dec 2013 Jan 2014".

I added an integration test that checks if the content is overflowing, so it should avoid regressions in the future. (at least for English).

PS: it will require updates to all locale files (I already did it for the ones we have on the repository).

Comment 22

[Miller Medeiros [:millermedeiros]]

Attachment: screenshot of PR #2

Comment 23

[Gareth Aye [:gaye] (back from PTO)]

Comment on attachment 8364152 [details] [review]
pull request #2

LGTM. I left a few nits on GH, but in general this is great :)

Comment 24

[Miller Medeiros [:millermedeiros]]

the visual refresh for v1.4+ will use the abbreviated style that I suggested on my PR. Just added the Bug 916411 as a blocker since it fixes intermittent build failures on travis-ci. I also updated the pull request (rebased, changed based on Gareth comments and fixed jshint errors).

Comment 25

[Miller Medeiros [:millermedeiros]]

merged into master https://github.com/mozilla-b2g/gaia/commit/4adf720865c7c7fd3aa3b1af4d0f4083ff39cdb6