Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(475)

Unified Diff: ui/views/window/dialog_client_view.cc

Issue 2807653002: Ensure default dialog button focus remains after a dialog update. (Closed)
Patch Set: Using a view deletion observer to track focus. Created 3 years, 7 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: ui/views/window/dialog_client_view.cc
diff --git a/ui/views/window/dialog_client_view.cc b/ui/views/window/dialog_client_view.cc
index 5d543a7572efe2b85b8772f293c766cbdb19430f..f7096064a8f2cf3214478de36355beee767129e2 100644
--- a/ui/views/window/dialog_client_view.cc
+++ b/ui/views/window/dialog_client_view.cc
@@ -17,10 +17,39 @@
#include "ui/views/controls/button/md_text_button.h"
#include "ui/views/layout/grid_layout.h"
#include "ui/views/style/platform_style.h"
+#include "ui/views/view_observer.h"
#include "ui/views/views_delegate.h"
#include "ui/views/widget/widget.h"
#include "ui/views/window/dialog_delegate.h"
+namespace {
sky 2017/05/17 19:08:30 Move this code to around line 80ish, inside the vi
ackermanb 2017/05/17 22:03:49 Done.
+
+class ViewDeletionObserver : public views::ViewObserver {
sky 2017/05/17 19:08:30 Add comment.
ackermanb 2017/05/17 22:03:50 Done.
+ public:
+ ViewDeletionObserver(views::View* observed_view)
sky 2017/05/17 19:08:30 explicit
ackermanb 2017/05/17 22:03:50 Done.
+ : observed_view_(observed_view) {
+ if (observed_view_)
+ observed_view_->AddObserver(this);
+ }
+
+ ~ViewDeletionObserver() override {
+ if (observed_view_)
+ observed_view_->RemoveObserver(this);
+ }
+
+ void OnViewIsDeleting(views::View* observed_view) override {
sky 2017/05/17 19:08:30 Prefix with: // ViewObserver:
ackermanb 2017/05/17 22:03:50 Done.
+ DCHECK_EQ(observed_view, observed_view_);
+ observed_view_ = nullptr;
sky 2017/05/17 19:08:30 Remove observer here.
ackermanb 2017/05/17 22:03:49 Done.
+ }
+
+ views::View* focused_view() { return observed_view_; }
+
+ private:
+ views::View* observed_view_ = nullptr;
Devlin 2017/05/17 18:13:49 DISALLOW_COPY_AND_ASSIGN()
ackermanb 2017/05/17 22:03:50 Done.
+};
+
+} // namespace
+
namespace views {
namespace {
@@ -319,6 +348,7 @@ void DialogClientView::SetupLayout() {
base::AutoReset<bool> auto_reset(&adding_or_removing_views_, true);
GridLayout* layout = new GridLayout(button_row_container_);
layout->set_minimum_size(minimum_size_);
+ ViewDeletionObserver deletion_observer(GetFocusManager()->GetFocusedView());
Devlin 2017/05/17 18:13:49 nit: Since we use the FocusManager twice in this f
ackermanb 2017/05/17 22:03:50 Done.
// Clobber any existing LayoutManager since it has weak references to child
// Views which may be removed by SetupViews().
@@ -399,6 +429,12 @@ void DialogClientView::SetupLayout() {
column_set->LinkColumnSizes(link[0], link[1], link[2], -1);
}
layout->AddPaddingRow(kFixed, insets.bottom());
+
+ // The default focus is lost when child views are added back into the dialog.
+ // This restores focus if the button is still available.
+ View* focused_view = deletion_observer.focused_view();
sky 2017/05/17 19:08:30 Name this previously_focused_view.
ackermanb 2017/05/17 22:03:50 Done.
+ if (!GetFocusManager()->GetFocusedView() && focused_view)
Devlin 2017/05/17 18:13:49 drive-by nit: I'd invert these, since checking foc
sky 2017/05/17 19:08:30 Also, add a conditional of Contains(focused_view);
ackermanb 2017/05/17 22:03:49 Done.
ackermanb 2017/05/17 22:03:50 Done.
+ focused_view->RequestFocus();
}
void DialogClientView::SetupViews() {

Powered by Google App Engine
This is Rietveld 408576698