UX Request: Edit Bookmark Invalid URL |
||||||||||||||||||
Issue descriptionWhen editing a Bookmark there's an URL validation while typing. If the URL is invalid a message is displayed as we can see on this screenshot: https://drive.google.com/open?id=1-rmvprIMfXp9E3e-ML1s-opSAo9jkUOw We currently don't have a mock for this scenario. Temporarily I'm just thinking of changing the font color to default red, like this: https://drive.google.com/open?id=16wb823AEestx0UBHl0eqlm6F0veHwoG1 Please let me know what you think we should do here. I actually like the red font approach (with a better red hue of course), but I know it can be kind of confusing.
,
May 18 2018
I like the red tint treatment too but it is no good for a11y, so I think we should also add a footer view label (same red tint) for that tableview section that uses the same "Invalid URL" string as the legacy design. Assigning to Martijn, who can provide label metrics (Martijn, for the red tint, use the same color specified for the close tab buttons in the tab switcher contextual (longpress) menus.
,
May 19 2018
Sg, this is what I have so far https://drive.google.com/file/d/1TIxoFEtyYtTp1KNEaszfPjEh9t7zziTl/view Will make any changes once I get the color tint and the label metrics.
,
May 22 2018
Hey Sergio, thanks for the quick turnaround. Actually I was talking about creating something akin to a UITableViewHeaderFooterView that sits below and **outside** the cell. Seeing as the last cell is the one we want to validate, it works out well to use a footer: https://i.stack.imgur.com/CuOb9.png
,
May 22 2018
Ooohh, cool sounds good! I'd like to wait on some mocks for this, so we don't have to do a followup clean up for colors, sizes, etc. And I don't think we have something similar right now. Once we have those I'll give this another look.
,
May 23 2018
Just a side note: color should never be the sole indicator of state. Many people are colorblind and will miss the cue entirely.
,
May 29 2018
Attached a preview, the specs are here: https://folio.googleplex.com/ntp/Bookmarks Also just want to point out that that the user input on focused input fields should be tinted black, as the Mock shows. This applies to all input fields.
,
Jun 26 2018
sczs should this be reassigned to you? What's the status?
,
Jun 26 2018
Assigning to myself, I've already implemented this on the cell, but we need to move it to the footer instead. So currently it would be considered UI Polishing
,
Jul 9
,
Jul 10
,
Jul 10
,
Jul 19
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8376eb4ac43c892bd7475bb26282cfe7480dfefb commit 8376eb4ac43c892bd7475bb26282cfe7480dfefb Author: sczs <sczs@chromium.org> Date: Thu Jul 19 06:24:25 2018 [ios] Moves EditBookmark InvalidURL label to footer. As per UX request, this is being moved to a tableView footer instead of the cell itself. Screenshot: https://drive.google.com/open?id=1zK2X9e-__l3CurjcB7Z3zgUIxTgSxXuJ Bug: 843827 Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet Change-Id: I977a625d10c108efa4f0794a2aafd77a32b8e495 Reviewed-on: https://chromium-review.googlesource.com/1142527 Reviewed-by: edchin <edchin@chromium.org> Commit-Queue: edchin <edchin@chromium.org> Cr-Commit-Position: refs/heads/master@{#576394} [modify] https://crrev.com/8376eb4ac43c892bd7475bb26282cfe7480dfefb/ios/chrome/browser/ui/bookmarks/bookmark_edit_view_controller.mm [modify] https://crrev.com/8376eb4ac43c892bd7475bb26282cfe7480dfefb/ios/chrome/browser/ui/bookmarks/cells/bookmark_text_field_item.h [modify] https://crrev.com/8376eb4ac43c892bd7475bb26282cfe7480dfefb/ios/chrome/browser/ui/bookmarks/cells/bookmark_text_field_item.mm
,
Jul 19
,
Jul 25
,
Jul 31
Verified the issue on the build 70.0.3508.0 canary. Invalid URL text in Edit Bookmark is seen on the devices iPhone7+(iOS 11.4),iPhone6+(12). Image : https://drive.google.com/file/d/173aCLhu1lBB9BTUoWjdHsMAlB5ekx1ZG/view?usp=sharing Invalid url text is not seen on the device iPhone6(iOS 10.3.3),iPhone6+(10.3.3) Image : https://drive.google.com/file/d/1XPU37DGUxgocQKlczvUobJ9NwccYAnQa/view?usp=sharing
,
Jul 31
Yep, not showing on iOS10 devices. Will take a look
,
Aug 1
Sent https://chromium-review.googlesource.com/c/chromium/src/+/1157525 to fix this, is a simple fix and should be safe to cherrypick
,
Aug 1
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/df7f5002d1f3333f7e1d5b417d29e295103b23bd commit df7f5002d1f3333f7e1d5b417d29e295103b23bd Author: sczs <sczs@chromium.org> Date: Wed Aug 01 19:34:49 2018 [ios] Configures BookmarkEditVC for dynamic footer height. On iOS 10 the footer wasn't being loaded since the tableView sectionFooterHeight and estimatedSectionFooterHeight properties weren't being set. Because of this the footerForSection method wasn't getting called. This CL also updates tableView.estimatedRowHeight to a more adequate value of 50. Bug: 843827 Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet Change-Id: Ia3c17df5ab3c4d5aad4d2a391bbd804dd6a0aec0 Reviewed-on: https://chromium-review.googlesource.com/1157525 Reviewed-by: Kurt Horimoto <kkhorimoto@chromium.org> Reviewed-by: edchin <edchin@chromium.org> Commit-Queue: Sergio Collazos <sczs@chromium.org> Cr-Commit-Position: refs/heads/master@{#579902} [modify] https://crrev.com/df7f5002d1f3333f7e1d5b417d29e295103b23bd/ios/chrome/browser/ui/bookmarks/bookmark_edit_view_controller.mm
,
Aug 1
Waiting for the next canary for verification
,
Aug 2
This bug requires manual review: Less than 29 days to go before AppStore submit on M69 Please contact the milestone owner if you have questions. Owners: amineer@(Android), kariahda@(iOS), cindyb@(ChromeOS), govind@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Aug 2
Issue verified Version: Chrome Canary 70.0.3510.0 Device: iPad iOS: 10.3.3 Correct "Invalid URL" on iOS 10 https://drive.google.com/open?id=1mPidBv13Ys2Cavrgu16OyIfGujaxiV0r https://drive.google.com/open?id=1ocKqGr5XEI59M7zcXdw9n8hfmViCBK8E https://drive.google.com/open?id=17kGSdPPsCc6Zx3FrRE1an0mB1r6MGFlq
,
Aug 2
Thanks yangulo@! To Kariah for manual approval
,
Aug 2
This bug requires manual review: Less than 29 days to go before AppStore submit on M69 Please contact the milestone owner if you have questions. Owners: amineer@(Android), kariahda@(iOS), cindyb@(ChromeOS), govind@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Aug 2
,
Aug 2
Approved.
,
Aug 2
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9a5adb41acff6b0eb683216866ebb5d91335514c commit 9a5adb41acff6b0eb683216866ebb5d91335514c Author: sczs <sczs@chromium.org> Date: Thu Aug 02 20:41:39 2018 [ios] Configures BookmarkEditVC for dynamic footer height. On iOS 10 the footer wasn't being loaded since the tableView sectionFooterHeight and estimatedSectionFooterHeight properties weren't being set. Because of this the footerForSection method wasn't getting called. This CL also updates tableView.estimatedRowHeight to a more adequate value of 50. TBR=sczs@chromium.org (cherry picked from commit df7f5002d1f3333f7e1d5b417d29e295103b23bd) Bug: 843827 Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet Change-Id: Ia3c17df5ab3c4d5aad4d2a391bbd804dd6a0aec0 Reviewed-on: https://chromium-review.googlesource.com/1157525 Reviewed-by: Kurt Horimoto <kkhorimoto@chromium.org> Reviewed-by: edchin <edchin@chromium.org> Commit-Queue: Sergio Collazos <sczs@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#579902} Reviewed-on: https://chromium-review.googlesource.com/1161201 Reviewed-by: Sergio Collazos <sczs@chromium.org> Cr-Commit-Position: refs/branch-heads/3497@{#350} Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753} [modify] https://crrev.com/9a5adb41acff6b0eb683216866ebb5d91335514c/ios/chrome/browser/ui/bookmarks/bookmark_edit_view_controller.mm
,
Aug 8
On iOS 10.3.3 - https://drive.google.com/file/d/1VnidT718DEI1RLAY7hyiXLyKbAgWXQKM/view?usp=sharing On iOS 11.4 - https://drive.google.com/file/d/1NhCljOjc0D99BzsqweTheRYluSkKJIbW/view?usp=sharing Verified the issue on the build 69.0.3497.31 Beta Invalid URL text in Edit Bookmark is seen on the devices iPhoneX iOS 11.4, iPhone 7+iOS 10.3.3, iPad Pro 12'9 iOS 11.4, iOS 10.3.3 |
||||||||||||||||||
►
Sign in to add a comment |
||||||||||||||||||
Comment 1 by kkhorimoto@chromium.org
, May 17 2018