Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2805683002:
Show "with <something>" for federated credentials in the MD settings. (Closed)
|
Created:
3 years, 8 months ago by vasilii Modified:
3 years, 8 months ago CC:
chromium-reviews, michaelpg+watch-md-settings_chromium.org, michaelpg+watch-md-ui_chromium.org, dbeam+watch-settings_chromium.org, stevenjb+watch-md-settings_chromium.org, arv+watch_chromium.org Target Ref:
refs/heads/master Project:
chromium Visibility:
Public. |
DescriptionShow "with <something>" for federated credentials in the MD settings.
A screenshot is attached to the bug.
BUG=706310
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
Review-Url: https://codereview.chromium.org/2805683002
Cr-Commit-Position: refs/heads/master@{#466400}
Committed: https://chromium.googlesource.com/chromium/src/+/7a0ea2253643c495276d4f5953871318f706509a
Patch Set 1 #
Total comments: 2
Patch Set 2 : align #
Total comments: 2
Messages
Total messages: 21 (10 generated)
Description was changed from ========== Show "with <something>" for federated credentials in the MD settings. A screenshot is attached to the bug. BUG=706310 ========== to ========== Show "with <something>" for federated credentials in the MD settings. A screenshot is attached to the bug. BUG=706310 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ==========
The CQ bit was checked by vasilii@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
vasilii@chromium.org changed reviewers: + hcarmona@chromium.org
Hi Hector, please review.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: ios-simulator on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
I added bettes@ to the bug to weigh in on design. Right now it looks like the text is too long and pushes the username to the left. We may want to elide the text. Also, clicking the overflow menu has an option for "View details" that shows an empty password field in the dialog.
Right. The dialog I can't resolve without a designer while the list is straightforward.
On 2017/04/06 17:00:36, vasilii wrote: > Right. The dialog I can't resolve without a designer while the list is > straightforward. Sounds good, we can update the dialog in a 2nd CL. Can you update this one so the columns line up along the left? You might need to elide the federated text.
https://codereview.chromium.org/2805683002/diff/1/chrome/browser/resources/se... File chrome/browser/resources/settings/passwords_and_forms_page/passwords_section.html (right): https://codereview.chromium.org/2805683002/diff/1/chrome/browser/resources/se... chrome/browser/resources/settings/passwords_and_forms_page/passwords_section.html:120: <div class="password-column"> Adding text-elide to the password-column makes text eliding work for me: http://screen/Ze5LuU4pTFd
A new screenshot is attached. https://codereview.chromium.org/2805683002/diff/1/chrome/browser/resources/se... File chrome/browser/resources/settings/passwords_and_forms_page/passwords_section.html (right): https://codereview.chromium.org/2805683002/diff/1/chrome/browser/resources/se... chrome/browser/resources/settings/passwords_and_forms_page/passwords_section.html:120: <div class="password-column"> On 2017/04/20 19:13:40, hcarmona wrote: > Adding text-elide to the password-column makes text eliding work for me: > http://screen/Ze5LuU4pTFd Done.
LGTM
The CQ bit was checked by vasilii@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 20001, "attempt_start_ts": 1492794113842910,
"parent_rev": "9f74925fb7302fe076090d56c4bd3aaa2207e173", "commit_rev":
"7a0ea2253643c495276d4f5953871318f706509a"}
Message was sent while issue was closed.
Description was changed from ========== Show "with <something>" for federated credentials in the MD settings. A screenshot is attached to the bug. BUG=706310 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ========== to ========== Show "with <something>" for federated credentials in the MD settings. A screenshot is attached to the bug. BUG=706310 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation Review-Url: https://codereview.chromium.org/2805683002 Cr-Commit-Position: refs/heads/master@{#466400} Committed: https://chromium.googlesource.com/chromium/src/+/7a0ea2253643c495276d4f595387... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://chromium.googlesource.com/chromium/src/+/7a0ea2253643c495276d4f595387...
Message was sent while issue was closed.
dbeam@chromium.org changed reviewers: + dbeam@chromium.org
Message was sent while issue was closed.
https://codereview.chromium.org/2805683002/diff/20001/chrome/browser/resource... File chrome/browser/resources/settings/passwords_and_forms_page/passwords_section.html (right): https://codereview.chromium.org/2805683002/diff/20001/chrome/browser/resource... chrome/browser/resources/settings/passwords_and_forms_page/passwords_section.html:127: <template is="dom-if" if="{{item.federationText}}"> why are you using {{}} (2-way binding) instead of [[]] (1-way)?
Message was sent while issue was closed.
https://codereview.chromium.org/2805683002/diff/20001/chrome/browser/resource... File chrome/browser/resources/settings/passwords_and_forms_page/passwords_section.html (right): https://codereview.chromium.org/2805683002/diff/20001/chrome/browser/resource... chrome/browser/resources/settings/passwords_and_forms_page/passwords_section.html:127: <template is="dom-if" if="{{item.federationText}}"> On 2017/04/21 22:16:39, Dan Beam wrote: > why are you using {{}} (2-way binding) instead of [[]] (1-way)? Because it was the only syntax I knew when I wrote this line :) |
