New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 898596 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Oct 25
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome , Mac
Pri: 1
Type: Bug



Sign in to add a comment

[Merge Request M71] Remove checkbox if migration dialog has only one card

Project Member Reported by siyua@chromium.org, Oct 24

Issue description

Requesting to merge the CL removing checkbox if migration dialog has only one card.

https://chromium-review.googlesource.com/c/chromium/src/+/1287434

Have manually tested successfully in Canary 72.0.3590.0. This CL is self-contained and touches some view files only related to the migration flow. The whole flow is controlled by Finch flag. 

This change fix the issue that there was a checkbox when migration dialog had only one card. It improves the UX since the checkbox in that case does not make sense. 

Thanks!
 
Cc: gov...@chromium.org
Owner: siyua@chromium.org
Project Member

Comment 2 by sheriffbot@chromium.org, Oct 25

Labels: -Merge-Request-71 Hotlist-Merge-Review Merge-Review-71
This bug requires manual review: M71 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: benmason@(Android), kariahda@(iOS), kbleicher@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

How is the change looking in canary?
Tested in the latest Canary on Mac (72.0.3591.0), it is working as expected for one card case (no checkbox) and multiple card case (have checkbox)
See attached screenshots.
One card.png
130 KB View Download
Multiple card.png
143 KB View Download
Labels: -Merge-Review-71 Merge-Approved-71
Approving merge to M71 branch 3578 based on comment #0 and #4. Please merge ASAP, Thank you.
Project Member

Comment 6 by bugdroid1@chromium.org, Oct 25

Labels: -merge-approved-71 merge-merged-3578
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/09dbf1ab7f9c193ef690210351703565417b2caf

commit 09dbf1ab7f9c193ef690210351703565417b2caf
Author: siyua <siyua@chromium.org>
Date: Thu Oct 25 16:35:53 2018

Remove checkbox if migration dialog has only one card

And also remove card index since we are not using index
for checkbox status update.

Bug:  898596 
Change-Id: I60513831b0392c469d62dfd45a15719670662272
Reviewed-on: https://chromium-review.googlesource.com/c/1287434
Reviewed-by: Evan Stade <estade@chromium.org>
Commit-Queue: Siyu An <siyua@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#602144}(cherry picked from commit a68a330f7c1a210c5277219d63fd55074163afd2)
Reviewed-on: https://chromium-review.googlesource.com/c/1299538
Reviewed-by: Jared Saul <jsaul@google.com>
Cr-Commit-Position: refs/branch-heads/3578@{#321}
Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}
[modify] https://crrev.com/09dbf1ab7f9c193ef690210351703565417b2caf/chrome/browser/ui/views/autofill/local_card_migration_dialog_view.cc
[modify] https://crrev.com/09dbf1ab7f9c193ef690210351703565417b2caf/chrome/browser/ui/views/autofill/migratable_card_view.cc
[modify] https://crrev.com/09dbf1ab7f9c193ef690210351703565417b2caf/chrome/browser/ui/views/autofill/migratable_card_view.h

Status: Fixed (was: Assigned)
Merge complete; marking as fixed.
Labels: Merge-Merged-71-3578
The following revision refers to this bug: 
https://chromium.googlesource.com/chromium/src.git/+/09dbf1ab7f9c193ef690210351703565417b2caf

Commit: 09dbf1ab7f9c193ef690210351703565417b2caf
Author: siyua@chromium.org
Commiter: jsaul@google.com
Date: 2018-10-25 16:35:53 +0000 UTC

Remove checkbox if migration dialog has only one card

And also remove card index since we are not using index
for checkbox status update.

Bug:  898596 
Change-Id: I60513831b0392c469d62dfd45a15719670662272
Reviewed-on: https://chromium-review.googlesource.com/c/1287434
Reviewed-by: Evan Stade <estade@chromium.org>
Commit-Queue: Siyu An <siyua@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#602144}(cherry picked from commit a68a330f7c1a210c5277219d63fd55074163afd2)
Reviewed-on: https://chromium-review.googlesource.com/c/1299538
Reviewed-by: Jared Saul <jsaul@google.com>
Cr-Commit-Position: refs/branch-heads/3578@{#321}
Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}
Cc: krajshree@chromium.org
Labels: Needs-Feedback
siyua@ - Could you please provide manual reproducible steps to verify the fix from our end.

Thanks...!!
Labels: TE-Verified-M71 TE-Verified-M71.0.3578.30
Tested the issue on Windows-10, Debian Rodete and Mac OS 10.13.6 using chrome latest Beta M71-71.0.3578.30 by following steps mentioned in the original comment. Observed that remove check box is displaying as expected. Hence adding TE-Verified label.

Please find the screenshot for reference.

Thank you!
898596(Mac).png
964 KB View Download
898596(Linux).png
291 KB View Download
898596(win).PNG
213 KB View Download
Labels: -Needs-Feedback

Sign in to add a comment