Commit e58702c5 authored by Elly Fong-Jones's avatar Elly Fong-Jones Committed by Commit Bot

views: use dialog buttons for desktopmedia initial focus

Previously initial focus in this dialog was the source list itself,
which was weird - the source list was focusable but had no keyboard
behavior or focus ring. An earlier change
(28f16f08) made it unfocusable but left
it as the initial focus view. When the initial focus view is
unfocusable, Widget::SetInitialFocus does not activate the widget at
all, which left it unusable with the keyboard. Oops!

This change drops initial focus on the dialog's cancel button. In
principle it would be nicer to drop focus on the first source in the
list, except that:
1) These lists are constructed asynchronously after the dialog appears
   so that view is not yet present when GetInitiallyFocusedView is
   called, and
2) Focusing one of the source views serves to select it, which would be
   surprising

Failing that it would be nice to use the dialog's default button, but
the default button is OK, and the OK button is disabled until the
user selects a source. We could change the default button to be Cancel
but on Mac Cancel cannot be the default button :(. As a result, this
change uses the Cancel button as the initially focused view but leaves
Ok as the default button.

Bug: 1007174
Change-Id: Ibc435b830394895551c99fd0fb4172729d981547
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1825321
Commit-Queue: Elly Fong-Jones <ellyjones@chromium.org>
Reviewed-by: default avatarPeter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#700345}
parent 3e295f0c
......@@ -91,10 +91,6 @@ void DesktopMediaListController::SetThumbnailSize(const gfx::Size& size) {
media_list_->SetThumbnailSize(size);
}
views::View* DesktopMediaListController::GetViewForInitialFocus() {
return view_;
}
void DesktopMediaListController::OnSourceAdded(DesktopMediaList* list,
int index) {
if (view_) {
......
......@@ -89,10 +89,6 @@ class DesktopMediaListController : public DesktopMediaListObserver,
void OnSourceSelectionChanged();
void AcceptSource();
// Returns the view that should receive initial focus in the dialog if this
// controller's source list is the one being shown.
views::View* GetViewForInitialFocus();
// These methods are used by the view (or its subviews) to query and
// update the underlying DesktopMediaList.
size_t GetSourceCount() const;
......
......@@ -338,7 +338,7 @@ bool DesktopMediaPickerDialogView::IsDialogButtonEnabled(
}
views::View* DesktopMediaPickerDialogView::GetInitiallyFocusedView() {
return list_controllers_.front()->GetViewForInitialFocus();
return GetDialogClientView()->cancel_button();
}
int DesktopMediaPickerDialogView::GetDefaultDialogButton() const {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment