22.88 KB, image/png
13.26 KB, image/png
355 bytes, text/html
46 bytes, text/x-github-pull-request
|Details | Review | Splinter Review|
25.88 KB, image/png
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.
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.
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"
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
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.
(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.
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: ? → +
Priority: -- → P3
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.
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.
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
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: [firstname.lastname@example.org][LOE:S] → [email@example.com][LOE:S]leorun1
James, did Eike's update in January receive any feedback?
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
Created attachment 750844 [details] Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/9830 Pointer to Github pull-request
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
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
Whiteboard: [firstname.lastname@example.org][LOE:S]leorun1 → [email@example.com][LOE:S]leorun1 LocRun1.2
Whiteboard: [firstname.lastname@example.org][LOE:S]leorun1 LocRun1.2 → [email@example.com][LOE:S]leorun1 LocRun1.2[priority][p=2]
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)
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+
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).
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.