Follow-up of bug 1142171. Our empty state for the history panel says: "Websites you visited most recently show up here.". This is not 100% correct for the new split-panel view introduced in bug 1142171. A history category can be empty because website have not been visited in this time frame.

I'd like to pick this up.

Review commit: https://reviewboard.mozilla.org/r/29329/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/29329/

Comment on attachment 8703251 [details] MozReview Request: Bug 1205236 - History Panel: A more appropriate empty state title. r+sebastian Review request updated; see interdiff: https://reviewboard.mozilla.org/r/29329/diff/1-2/

Comment on attachment 8703295 [details] Screenshot Flagging antlam for UX feedback

Comment on attachment 8703251 [details] MozReview Request: Bug 1205236 - History Panel: A more appropriate empty state title. r+sebastian https://reviewboard.mozilla.org/r/29329/#review26355 ::: mobile/android/base/java/org/mozilla/gecko/home/HistoryPanel.java:98 (Diff revision 2) > + private boolean isCursorEmpty = false; Do we need this? This boolean is only checked in resetEmptyText() - Shouldn't this only be called if the cursor is empty? ::: mobile/android/base/java/org/mozilla/gecko/home/HistoryPanel.java:335 (Diff revision 2) > + private String getEmptyText() { > + String result = getMostRecentSectionTitle(selected); > + if (result.matches(".*7.*")) { > + result = (getString(R.string.in_the) + " " + result).toLowerCase(); > + } > + else if (selected.toString().matches(".*SIX_MONTH.*")) { > + result = getString(R.string.history_six_months_ago); > + } > + else if (selected.toString().matches(".*MONTH.*")) { > + result = getString(R.string.in) + " " + result; > + } > + else { > + result = result.toLowerCase(); > + } > + return result; > + } This sentence composition works for english but is not going to work in a lot of other languages. You will probably have to create separate strings like we did for the sections.

Comment on attachment 8703295 [details] Screenshot heh, nice!

Comment on attachment 8703251 [details] MozReview Request: Bug 1205236 - History Panel: A more appropriate empty state title. r+sebastian Review request updated; see interdiff: https://reviewboard.mozilla.org/r/29329/diff/2-3/

https://reviewboard.mozilla.org/r/29329/#review26355 > Do we need this? This boolean is only checked in resetEmptyText() - Shouldn't this only be called if the cursor is empty? Ah. I did not see that updateUiFromCursor() gets called after cursor transitions. I was also calling it in the load() method as well.

Comment on attachment 8703251 [details] MozReview Request: Bug 1205236 - History Panel: A more appropriate empty state title. r+sebastian https://reviewboard.mozilla.org/r/29329/#review27023 ::: mobile/android/base/java/org/mozilla/gecko/home/HistoryPanel.java:342 (Diff revision 3) > + result.toLowerCase()); You are manually inserting today/yesterday/most recently into the string and run toLowerCase() on it. I'm not sure if this will create a valid translation for all languages. Let's bring in Axel and use his experience. :)

(In reply to Sebastian Kaspari (:sebastian) from comment #12) > Comment on attachment 8703251 [details] > MozReview Request: Bug 1205236 - History Panel: Show section title in empty > state. r?sebastian > > https://reviewboard.mozilla.org/r/29329/#review27023 > > ::: mobile/android/base/java/org/mozilla/gecko/home/HistoryPanel.java:342 > (Diff revision 3) > > + result.toLowerCase()); > > You are manually inserting today/yesterday/most recently into the string and > run toLowerCase() on it. I'm not sure if this will create a valid > translation for all languages. Let's bring in Axel and use his experience. :) @Axel: For this screen[1] we basically have the following variations of strings: * Websites you visited <today / yesterday / most recently> show up here. * Websites you visited in <January / February / March / ..> show up here. * Websites you visited in the last seven days show up here. * Websites you visited more than six months ago show up here. What's the best way to split them up for translating? Can we insert the names of months dynamically? How about "today", "yesterday" and "most recently"? [1] https://bugzilla.mozilla.org/attachment.cgi?id=8703295

(In reply to Sebastian Kaspari (:sebastian) from comment #13) > * Websites you visited <today / yesterday / most recently> show up here. I would go with 3 separate strings here (keeping in mind the end of this comment). > * Websites you visited in <January / February / March / ..> show up here. The current implementation definitely doesn't work: * Case: Italian (and several other European languages) don't use uppercase for a month name in the middle of a sentence. * Declension: I assume the month you get from Android is some sort of nominative, for example 'in X' would require an ablative form in Latin. Is there any chance to dramatically simplify this and display 'Websites you visited in the selected timeframe/period show up here."?

Comment on attachment 8703251 [details] MozReview Request: Bug 1205236 - History Panel: A more appropriate empty state title. r+sebastian Review request updated; see interdiff: https://reviewboard.mozilla.org/r/29329/diff/3-4/

Comment on attachment 8703251 [details] MozReview Request: Bug 1205236 - History Panel: A more appropriate empty state title. r+sebastian https://reviewboard.mozilla.org/r/29329/#review28145 r+ after addressing the mentioned points. Great patch! Much simpler now. :) ::: mobile/android/base/java/org/mozilla/gecko/home/HistoryPanel.java:313 (Diff revision 4) > + if(selected == null || mRangeAdapter == null || mRangeList == null) { NIT: if _space_ { ::: mobile/android/base/java/org/mozilla/gecko/home/HistoryPanel.java:315 (Diff revision 4) > + } > + else { NIT: } else { ::: mobile/android/base/locales/en-US/android_strings.dtd:525 (Diff revision 4) > +<!ENTITY home_selected_empty "Websites you visited in the selected timeframe/period show up here."> We probably should decide to use either timeframe or period here. Flag antlam if you want to get help making a decision.

Comment on attachment 8703251 [details] MozReview Request: Bug 1205236 - History Panel: A more appropriate empty state title. r+sebastian Review request updated; see interdiff: https://reviewboard.mozilla.org/r/29329/diff/4-5/