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

RESOLVED FIXED in 1.3 C3/1.4 S3(31jan)

Status

Firefox OS
Gaia::Calendar
P3
normal
RESOLVED FIXED
5 years ago
3 years ago

People

(Reporter: jsmith, Assigned: millermedeiros)

Tracking

({l12y})

unspecified
1.3 C3/1.4 S3(31jan)
ARM
Gonk (Firefox OS)
Dependency tree / graph

Firefox Tracking Flags

(blocking-basecamp:-, tracking-b2g:backlog, b2g18+, b2g-v1.2 affected)

Details

(Whiteboard: [mentor=jlal@mozilla.com][LOE:S]leorun1 LocRun1.2[priority][p=2])

Attachments

(5 attachments)

(Reporter)

Description

5 years ago
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.
(Reporter)

Updated

5 years ago
Keywords: polish
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.
Flags: needinfo?(kyee)
Whiteboard: [mentor=jlal@mozilla.com][LOE:S]
(Reporter)

Comment 2

5 years ago
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"
(Reporter)

Updated

5 years ago
Summary: Week view: Text is too much cut off in month title with November and December 2012 being the current week in view → 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
(Reporter)

Updated

5 years ago
Duplicate of this bug: 813916

Comment 4

5 years ago
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

5 years ago
(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.
Flags: needinfo?(kyee)
I think this should block, we needed to remove my band-aid that let us actually see the current month by using
blocking-basecamp: --- → ?
... 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.
blocking-basecamp: ? → +
Keywords: polish
Priority: -- → P3
Assignee: nobody → jlal

Updated

5 years ago
Target Milestone: --- → B2G C3 (12dec-1jan)

Comment 8

5 years ago
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.
(Reporter)

Comment 9

5 years ago
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.
blocking-basecamp: + → ?
tracking-b2g18: --- → ?
:) 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.
Assignee: jlal → Eike.send
blocking-basecamp: ? → -
tracking-b2g18: ? → +

Comment 11

5 years ago
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

5 years ago
Created attachment 733287 [details]
not shortened month names

Comment 13

5 years ago
Created attachment 740990 [details]
Dec-Jan Overlap 4-23-13

Comment 14

5 years ago
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
Whiteboard: [mentor=jlal@mozilla.com][LOE:S] → [mentor=jlal@mozilla.com][LOE:S]leorun1
James, did Eike's update in January receive any feedback?
Flags: needinfo?(jlal)
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.
Assignee: Eike.send → jlal
Flags: needinfo?(jlal)
Created attachment 750844 [details]
Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/9830

Pointer to Github pull-request
Attachment #750844 - Flags: review?(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.
Attachment #750844 - Flags: review?(kgrandon)
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...
Assignee: jlal → nobody

Comment 20

4 years ago
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
status-b2g-v1.2: --- → affected
Keywords: l12y
Whiteboard: [mentor=jlal@mozilla.com][LOE:S]leorun1 → [mentor=jlal@mozilla.com][LOE:S]leorun1 LocRun1.2

Updated

4 years ago
Blocks: 928174
blocking-b2g: --- → backlog
Whiteboard: [mentor=jlal@mozilla.com][LOE:S]leorun1 LocRun1.2 → [mentor=jlal@mozilla.com][LOE:S]leorun1 LocRun1.2[priority][p=2]
Target Milestone: B2G C3 (12dec-1jan) → ---
(Assignee)

Updated

4 years ago
Assignee: nobody → mmedeiros
Target Milestone: --- → 1.3 C3/1.4 S3(31jan)
(Assignee)

Comment 21

4 years ago
Created attachment 8364152 [details] [review]
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).
Attachment #8364152 - Flags: review?(gaye)
(Assignee)

Comment 22

4 years ago
Created attachment 8364159 [details]
screenshot of PR #2
(Assignee)

Updated

4 years ago
Blocks: 878476
Comment on attachment 8364152 [details] [review]
pull request #2

LGTM. I left a few nits on GH, but in general this is great :)
Attachment #8364152 - Flags: review?(gaye) → review+
(Assignee)

Updated

4 years ago
Depends on: 916411
(Assignee)

Comment 24

4 years ago
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).
(Assignee)

Comment 25

4 years ago
merged into master https://github.com/mozilla-b2g/gaia/commit/4adf720865c7c7fd3aa3b1af4d0f4083ff39cdb6
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
blocking-b2g: backlog → ---
tracking-b2g: --- → backlog
You need to log in before you can comment on or make changes to this bug.