Add contextual menu items for copying and showing parts of credentials in settings |
|||||||||||||
Issue descriptionAs tracked in bug 159166, password settings on iOS are being extended with the functionality to copy site and username, and to show and copy password values. The above is achievable with fixed buttons, but it should also be achievable through context menus, because that is the pattern on iOS. Also, as pschaffner@ put it: "Our explicit and visible controls arguably make this other mechanism unnecessary, but often when a foot trail exists, people will miss the roadsigns for the new highway." :) Attached is an old screenshot from the design doc for illustration. It is not necessarily a mock to be followed, that will need to be clarified at the time of implementation. If there is a precedent in Chrome on iOS for this, we should just follow that.
,
Jul 7 2017
Notes for myself, about how to implement this: UIMenuController seems to be what controls displaying menus. https://chromereviews.googleplex.com/266007013/ is how ioanap@ did this (but sadly that code did not land), which seems a big help in rediscovering how it should be done.
,
Jul 8 2017
Adding screenshots for https://chromium-review.googlesource.com/c/563695/. The call-outs (pop-ups) are shown on tap on the site URL, the username, or the password cell.
,
Jul 10 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1089d064be666f5f0437da78bdc5f875795dc7b6 commit 1089d064be666f5f0437da78bdc5f875795dc7b6 Author: Vaclav Brozek <vabr@chromium.org> Date: Mon Jul 10 13:23:46 2017 Add context menu items to password details settings view This adds the Copy context menu item to the cells with site, username and password, and also the Show/Hide item for password (whether it's Show or Hide depends on whether the password is masked or unmasked, respectively). Screenshots are at https://crbug.com/717968#c3 . BUG= 717968 Change-Id: Iaeec460deea8c117179a8469eb771e9802765012 Reviewed-on: https://chromium-review.googlesource.com/563695 Commit-Queue: Vaclav Brozek <vabr@chromium.org> Reviewed-by: Louis Romero <lpromero@chromium.org> Cr-Commit-Position: refs/heads/master@{#485252} [modify] https://crrev.com/1089d064be666f5f0437da78bdc5f875795dc7b6/ios/chrome/app/strings/ios_strings.grd [modify] https://crrev.com/1089d064be666f5f0437da78bdc5f875795dc7b6/ios/chrome/browser/ui/settings/password_details_collection_view_controller.mm [modify] https://crrev.com/1089d064be666f5f0437da78bdc5f875795dc7b6/ios/chrome/browser/ui/settings/passwords_settings_egtest.mm
,
Jul 10 2017
This looks good to me (mocks in #3). Thank you!
,
Jul 10 2017
Thanks! Closing for now, please feel free to reopen if there are additional comments.
,
Sep 22 2017
Verified (#3) on iOS 10.3.3, 11.0 on iPhone 6 +, iPhone 7+, iPad Pro 12'5 on build 62.0.3202.29 Beta. |
|||||||||||||
►
Sign in to add a comment |
|||||||||||||
Comment 1 by pschaffner@chromium.org
, May 5 2017Actually, that mock is accurate, so feel free to build from that :) "Show", of course, would be a toggle control ("Show" | "Hide"), just like the physical button.