Use UITextView for better bulleted list presentation

Use UITextView for better bulleted list presentation

BUG=724636
TEST=Navigating to a page that fails to load twice in a row should present a bulleted list of suggestions for the user to try. However this list on small screened devices, and in certain localizations can lay out the bulleted text in a non aligned manner.

Specifically iPhone 5/5S/5SE, in French, is known to do this.

This change should align the left of the text blocks, and not allow the text to wrap just to the bullet indentation.

Review-Url: https://codereview.chromium.org/2905953003
Cr-Commit-Position: refs/heads/master@{#475594}
Committed: https://chromium.googlesource.com/chromium/src/+/cff1e1836ec5a28bc4a31913ef6d22d62c9e2842

[PL, 2017-05-25 22:08:43]

peterlaurens@chromium.org changed reviewers:
+ kkhorimoto@chromium.org

[PL, 2017-05-25 22:08:44]

Hi Kurt!

pkl@ noticed a bad wrap with long lines on the sad tab view when we're showing a
bulleted list - a wrapping second line of text wraps under the bullets rather
than aligning with the first line of text.

We can do this fairly easily with tabstops in a UITextView, so I've replaced the
message label in sad tab with a message text view.

I've tried this in a few languages, including French where pkl@ saw the issue
originally. Also tried in an RTL language, and moving from portrait->landscape.

Thanks!

- Peter

[kkhorimoto, 2017-05-26 21:40:22]

lgtm

https://codereview.chromium.org/2905953003/diff/20001/ios/chrome/browser/ui/s...
File ios/chrome/browser/ui/sad_tab/sad_tab_view.mm (right):

https://codereview.chromium.org/2905953003/diff/20001/ios/chrome/browser/ui/s...
ios/chrome/browser/ui/sad_tab/sad_tab_view.mm:240: IDS_SAD_TAB_RELOAD_TRY)]];
Can we break this out into some local variables to make the formatting a little
better?

[pkl (ping after 24h if needed), 2017-05-26 21:51:16]

pkl@chromium.org changed reviewers:
+ pkl@chromium.org

[pkl (ping after 24h if needed), 2017-05-26 21:51:17]

Drive-by. Thank you for fixing this!

You should have a TEST= line to guide the testers what to look for.

https://codereview.chromium.org/2905953003/diff/20001/ios/chrome/browser/ui/s...
File ios/chrome/browser/ui/sad_tab/sad_tab_view.mm (right):

https://codereview.chromium.org/2905953003/diff/20001/ios/chrome/browser/ui/s...
ios/chrome/browser/ui/sad_tab/sad_tab_view.mm:176: NSMutableString*
bulletedString = [[NSMutableString string];
Looks like you have one too many (stray) [ here.

https://codereview.chromium.org/2905953003/diff/20001/ios/chrome/browser/ui/s...
ios/chrome/browser/ui/sad_tab/sad_tab_view.mm:252: break;
"break" should go inside the {} blocks, based on my read of C++ style guide
https://engdoc.corp.google.com/eng/doc/devguide/cpp/styleguide.shtml?cl=head#...

[PL, 2017-05-26 21:56:30]

Thanks for the guidance!

https://codereview.chromium.org/2905953003/diff/20001/ios/chrome/browser/ui/s...
File ios/chrome/browser/ui/sad_tab/sad_tab_view.mm (right):

https://codereview.chromium.org/2905953003/diff/20001/ios/chrome/browser/ui/s...
ios/chrome/browser/ui/sad_tab/sad_tab_view.mm:176: NSMutableString*
bulletedString = [[NSMutableString string];
On 2017/05/26 21:51:17, pkl -pls.ping.after.24h. wrote:
> Looks like you have one too many (stray) [ here.

Ha wow, thanks amazing spot! This was a last minute change I didn't compile
(obviously), I would like to change this back to [[NSMutableString alloc] init]
actually to avoid using the string convenience method as it's less definitive
what you will get back.

https://codereview.chromium.org/2905953003/diff/20001/ios/chrome/browser/ui/s...
ios/chrome/browser/ui/sad_tab/sad_tab_view.mm:240: IDS_SAD_TAB_RELOAD_TRY)]];
On 2017/05/26 21:40:22, kkhorimoto_ wrote:
> Can we break this out into some local variables to make the formatting a
little
> better?

Absolutely, will do, thanks!

https://codereview.chromium.org/2905953003/diff/20001/ios/chrome/browser/ui/s...
ios/chrome/browser/ui/sad_tab/sad_tab_view.mm:252: break;
On 2017/05/26 21:51:17, pkl -pls.ping.after.24h. wrote:
> "break" should go inside the {} blocks, based on my read of C++ style guide
>
https://engdoc.corp.google.com/eng/doc/devguide/cpp/styleguide.shtml?cl=head#...

Thanks! Will do.

[PL, 2017-05-30 17:48:23]

The CQ bit was checked by peterlaurens@chromium.org

[PL, 2017-05-30 17:48:23]

The patchset sent to the CQ was uploaded after l-g-t-m from
kkhorimoto@chromium.org
Link to the patchset: https://codereview.chromium.org/2905953003/#ps40001
(title: "Review feedback")

[commit-bot: I haz the power, 2017-05-30 17:48:46]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[PL, 2017-05-30 17:50:44]

Description was changed from

==========
Use UITextView for better bulleted list presentation

BUG=724636
==========

to

==========
Use UITextView for better bulleted list presentation

BUG=724636
TEST=Navigating to a page that fails to load twice in a row should present a
bulleted list of suggestions for the user to try. However this list on small
screened devices, and in certain localizations can lay out the bulleted text in
a non aligned manner.

Specifically iPhone 5/5S/5SE, in French, is known to do this.

This change should align the left of the text blocks, and not allow the text to
wrap just to the bullet indentation.
==========

[commit-bot: I haz the power, 2017-05-30 18:04:12]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1496166503508620,
"parent_rev": "4c386e823fb0576f66e5a79e7075c2f464a7b307", "commit_rev":
"cff1e1836ec5a28bc4a31913ef6d22d62c9e2842"}

[commit-bot: I haz the power, 2017-05-30 18:04:20]

Description was changed from

==========
Use UITextView for better bulleted list presentation

BUG=724636
TEST=Navigating to a page that fails to load twice in a row should present a
bulleted list of suggestions for the user to try. However this list on small
screened devices, and in certain localizations can lay out the bulleted text in
a non aligned manner.

Specifically iPhone 5/5S/5SE, in French, is known to do this.

This change should align the left of the text blocks, and not allow the text to
wrap just to the bullet indentation.
==========

to

==========
Use UITextView for better bulleted list presentation

BUG=724636
TEST=Navigating to a page that fails to load twice in a row should present a
bulleted list of suggestions for the user to try. However this list on small
screened devices, and in certain localizations can lay out the bulleted text in
a non aligned manner.

Specifically iPhone 5/5S/5SE, in French, is known to do this.

This change should align the left of the text blocks, and not allow the text to
wrap just to the bullet indentation.

Review-Url: https://codereview.chromium.org/2905953003
Cr-Commit-Position: refs/heads/master@{#475594}
Committed:
https://chromium.googlesource.com/chromium/src/+/cff1e1836ec5a28bc4a31913ef6d...
==========

[commit-bot: I haz the power, 2017-05-30 18:04:22]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/cff1e1836ec5a28bc4a31913ef6d...