Commit 38703614 authored by edchin's avatar edchin Committed by Commit Bot

[ios] Improve page placeholder placement

Previously, the grey page placeholder image attempted to use the
webProxyView's contentInset property to properly position the
placeholder. This used frame setting.

The preferred way to position content in the BVC is to use the
ContentArea NamedGuide with autolayout contraints.

Using the preferred autolayout + NamedGuide approach also fixes a
placeholder malpositioning bug related to fullscreen experiments.

Bug: 956077
Change-Id: Ia3d115ae203bacb59d41cdb3c377a4a75c00dc95
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1593921
Commit-Queue: edchin <edchin@chromium.org>
Reviewed-by: default avataredchin <edchin@chromium.org>
Reviewed-by: default avatarEugene But <eugenebut@chromium.org>
Reviewed-by: default avatarKurt Horimoto <kkhorimoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#657797}
parent ce99b3c2
...@@ -40,9 +40,11 @@ source_set("web") { ...@@ -40,9 +40,11 @@ source_set("web") {
"//ios/chrome/browser/browser_state", "//ios/chrome/browser/browser_state",
"//ios/chrome/browser/ntp", "//ios/chrome/browser/ntp",
"//ios/chrome/browser/snapshots", "//ios/chrome/browser/snapshots",
"//ios/chrome/browser/ui/commands:commands", "//ios/chrome/browser/ui/commands",
"//ios/chrome/browser/ui/elements",
"//ios/chrome/browser/ui/fullscreen", "//ios/chrome/browser/ui/fullscreen",
"//ios/chrome/browser/ui/util:util", "//ios/chrome/browser/ui/util",
"//ios/chrome/common/ui_util",
"//ios/net", "//ios/net",
"//ios/web", "//ios/web",
"//ios/web/common", "//ios/web/common",
...@@ -99,7 +101,7 @@ source_set("unit_tests") { ...@@ -99,7 +101,7 @@ source_set("unit_tests") {
":test_support", ":test_support",
":web", ":web",
":web_internal", ":web_internal",
"//base:base", "//base",
"//base/test:test_support", "//base/test:test_support",
"//components/search_engines", "//components/search_engines",
"//components/services/patch/public/interfaces", "//components/services/patch/public/interfaces",
...@@ -109,6 +111,7 @@ source_set("unit_tests") { ...@@ -109,6 +111,7 @@ source_set("unit_tests") {
"//ios/chrome/browser/browser_state:test_support", "//ios/chrome/browser/browser_state:test_support",
"//ios/chrome/browser/ntp", "//ios/chrome/browser/ntp",
"//ios/chrome/browser/snapshots", "//ios/chrome/browser/snapshots",
"//ios/chrome/browser/ui/util",
"//ios/chrome/test:test_support", "//ios/chrome/test:test_support",
"//ios/net:test_support", "//ios/net:test_support",
"//ios/web", "//ios/web",
...@@ -121,7 +124,7 @@ source_set("unit_tests") { ...@@ -121,7 +124,7 @@ source_set("unit_tests") {
"//testing/gmock", "//testing/gmock",
"//testing/gtest", "//testing/gtest",
"//third_party/ocmock", "//third_party/ocmock",
"//ui/base:base", "//ui/base",
"//url:url", "//url:url",
] ]
} }
......
...@@ -8,6 +8,7 @@ ...@@ -8,6 +8,7 @@
#import <UIKit/UIKit.h> #import <UIKit/UIKit.h>
#include "base/macros.h" #include "base/macros.h"
#import "ios/chrome/browser/ui/elements/top_aligned_image_view.h"
#include "ios/web/public/web_state/web_state_observer.h" #include "ios/web/public/web_state/web_state_observer.h"
#import "ios/web/public/web_state/web_state_user_data.h" #import "ios/web/public/web_state/web_state_user_data.h"
...@@ -62,7 +63,7 @@ class PagePlaceholderTabHelper ...@@ -62,7 +63,7 @@ class PagePlaceholderTabHelper
web::WebState* web_state_ = nullptr; web::WebState* web_state_ = nullptr;
// View used to display the placeholder. // View used to display the placeholder.
UIImageView* placeholder_view_ = nil; TopAlignedImageView* placeholder_view_ = nil;
// true if placeholder is currently being displayed. // true if placeholder is currently being displayed.
bool displaying_placeholder_ = false; bool displaying_placeholder_ = false;
......
...@@ -8,6 +8,8 @@ ...@@ -8,6 +8,8 @@
#include "base/logging.h" #include "base/logging.h"
#include "base/threading/thread_task_runner_handle.h" #include "base/threading/thread_task_runner_handle.h"
#import "ios/chrome/browser/snapshots/snapshot_tab_helper.h" #import "ios/chrome/browser/snapshots/snapshot_tab_helper.h"
#import "ios/chrome/browser/ui/util/named_guide.h"
#import "ios/chrome/common/ui_util/constraints_ui_util.h"
#import "ios/web/public/web_state/ui/crw_web_view_proxy.h" #import "ios/web/public/web_state/ui/crw_web_view_proxy.h"
#if !defined(__has_feature) || !__has_feature(objc_arc) #if !defined(__has_feature) || !__has_feature(objc_arc)
...@@ -83,11 +85,9 @@ void PagePlaceholderTabHelper::AddPlaceholder() { ...@@ -83,11 +85,9 @@ void PagePlaceholderTabHelper::AddPlaceholder() {
// Lazily create the placeholder view. // Lazily create the placeholder view.
if (!placeholder_view_) { if (!placeholder_view_) {
placeholder_view_ = [[UIImageView alloc] init]; placeholder_view_ = [[TopAlignedImageView alloc] init];
placeholder_view_.backgroundColor = [UIColor whiteColor]; placeholder_view_.backgroundColor = [UIColor whiteColor];
placeholder_view_.contentMode = UIViewContentModeScaleAspectFit; placeholder_view_.translatesAutoresizingMaskIntoConstraints = NO;
placeholder_view_.autoresizingMask =
UIViewAutoresizingFlexibleWidth | UIViewAutoresizingFlexibleHeight;
} }
// Update placeholder view's image and display it on top of WebState's view. // Update placeholder view's image and display it on top of WebState's view.
...@@ -112,15 +112,11 @@ void PagePlaceholderTabHelper::AddPlaceholder() { ...@@ -112,15 +112,11 @@ void PagePlaceholderTabHelper::AddPlaceholder() {
} }
void PagePlaceholderTabHelper::DisplaySnapshotImage(UIImage* snapshot) { void PagePlaceholderTabHelper::DisplaySnapshotImage(UIImage* snapshot) {
CGRect frame = web_state_->GetView().frame;
UIEdgeInsets inset = web_state_->GetWebViewProxy().contentInset;
frame.origin.x += inset.left;
frame.origin.y += inset.top;
frame.size.width -= (inset.right + inset.left);
frame.size.height -= (inset.bottom + inset.top);
placeholder_view_.frame = frame;
placeholder_view_.image = snapshot; placeholder_view_.image = snapshot;
[web_state_->GetView() addSubview:placeholder_view_]; [web_state_->GetView() addSubview:placeholder_view_];
AddSameConstraints([NamedGuide guideWithName:kContentAreaGuide
view:placeholder_view_],
placeholder_view_);
} }
void PagePlaceholderTabHelper::RemovePlaceholder() { void PagePlaceholderTabHelper::RemovePlaceholder() {
...@@ -130,7 +126,7 @@ void PagePlaceholderTabHelper::RemovePlaceholder() { ...@@ -130,7 +126,7 @@ void PagePlaceholderTabHelper::RemovePlaceholder() {
displaying_placeholder_ = false; displaying_placeholder_ = false;
// Remove placeholder view with a fade-out animation. // Remove placeholder view with a fade-out animation.
__weak UIImageView* weak_placeholder_view = placeholder_view_; __weak UIView* weak_placeholder_view = placeholder_view_;
[UIView animateWithDuration:kPlaceholderFadeOutAnimationLengthInSeconds [UIView animateWithDuration:kPlaceholderFadeOutAnimationLengthInSeconds
animations:^{ animations:^{
weak_placeholder_view.alpha = 0.0f; weak_placeholder_view.alpha = 0.0f;
......
...@@ -11,6 +11,7 @@ ...@@ -11,6 +11,7 @@
#import "base/test/ios/wait_util.h" #import "base/test/ios/wait_util.h"
#include "ios/chrome/browser/browser_state/test_chrome_browser_state.h" #include "ios/chrome/browser/browser_state/test_chrome_browser_state.h"
#import "ios/chrome/browser/snapshots/snapshot_tab_helper.h" #import "ios/chrome/browser/snapshots/snapshot_tab_helper.h"
#import "ios/chrome/browser/ui/util/named_guide.h"
#import "ios/web/public/test/fakes/test_web_state.h" #import "ios/web/public/test/fakes/test_web_state.h"
#include "ios/web/public/test/test_web_thread_bundle.h" #include "ios/web/public/test/test_web_thread_bundle.h"
#include "testing/gtest/include/gtest/gtest.h" #include "testing/gtest/include/gtest/gtest.h"
...@@ -36,6 +37,10 @@ class PagePlaceholderTabHelperTest : public PlatformTest { ...@@ -36,6 +37,10 @@ class PagePlaceholderTabHelperTest : public PlatformTest {
web_state_view_.backgroundColor = [UIColor blueColor]; web_state_view_.backgroundColor = [UIColor blueColor];
web_state_->SetView(web_state_view_); web_state_->SetView(web_state_view_);
// The Content Area named guide should be available.
NamedGuide* guide = [[NamedGuide alloc] initWithName:kContentAreaGuide];
[web_state_view_ addLayoutGuide:guide];
// PagePlaceholderTabHelper uses SnapshotTabHelper, so ensure it has been // PagePlaceholderTabHelper uses SnapshotTabHelper, so ensure it has been
// created. // created.
SnapshotTabHelper::CreateForWebState(web_state_.get(), SnapshotTabHelper::CreateForWebState(web_state_.get(),
......
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