[WIP][iOS] Create a facepile implementation in Chrome [chromium/src : main]
Gauthier Ambard (Gerrit)
Gauthier Ambard added 1 comment![]( "Open in Gerrit")
PTAL, not fully ready for review yet, but comments on the approach are welcome
Related details
- Aliona Dangla
- Ewann Pellé
- Louis Romero
Code-Coverage
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Ewann Pellé (Gerrit)
Ewann Pellé added 14 comments![]( "Open in Gerrit")
// to its consumer (typically a FacePileView).optional nit: `FacePileView consumer`
raw_ptr<data_sharing::DataSharingService> _dataSharingService;optional nit: add comments for group of ivars.
e.g
```
@implementation FacePileMediator {
// Services required by the mediator.
raw_ptr<data_sharing::DataSharingService> _dataSharingService;
raw_ptr<ShareKitService> _shareKitService;
raw_ptr<signin::IdentityManager> _identityManager;
// Settings specific to this mediator instance.
FacePileConfiguration* _configuration;
// Handles observing changes from services.
std::unique_ptr<DataSharingServiceObserverBridge> _dataSharingServiceObserver;
std::unique_ptr<ScopedDataSharingSyncObservation> _scopedDataSharingServiceObservation;
}
```
_configuration = configuration;
_dataSharingService = dataSharingService;
_shareKitService = shareKitService;
_identityManager = identityManager;Some services can be nullptr ?
if (!_consumer || !_dataSharingService->IsGroupDataModelLoaded()) {
return;
}
CoreAccountInfo account =
_identityManager->GetPrimaryAccountInfo(signin::ConsentLevel::kSignin);
if (account.IsEmpty()) {
// No current logged in user.
return;
}
```suggestion
if (!_consumer || !_dataSharingService->IsGroupDataModelLoaded()) {
return;
}
CoreAccountInfo account =
_identityManager->GetPrimaryAccountInfo(signin::ConsentLevel::kSignin);
if (account.IsEmpty()) {
// No current logged in user.
return;
}
```
ShareKitAvatarConfiguration* ownerConfig;
ShareKitAvatarConfiguration* secondConfig;
ShareKitAvatarConfiguration* thirdConfig;
BOOL foundSelf = NO;
BOOL needThirdFace = groupData->members.size() == 3;
for (data_sharing::GroupMember member : groupData->members) {
if (ownerConfig && secondConfig && foundSelf &&
(!needThirdFace || thirdConfig)) {
break;
}
BOOL isOwner = (member.role == data_sharing::MemberRole::kOwner);
BOOL isSelf = (member.gaia_id == account.gaia);
foundSelf = foundSelf || isSelf;
if (!isOwner && !isSelf && secondConfig &&
(!needThirdFace || thirdConfig)) {
// If we already have a second/third config, there is only a need to
// update the configs when it is self or the owner.
continue;
}
ShareKitAvatarConfiguration* config =
[[ShareKitAvatarConfiguration alloc] init];
config.avatarUrl = net::NSURLWithGURL(member.avatar_url);
config.displayName = base::SysUTF8ToNSString(member.given_name);
config.avatarSize =
CGSizeMake(_configuration.avatarSize, _configuration.avatarSize);
if (isOwner) {
ownerConfig = config;
} else if (isSelf) {
if (needThirdFace) {
thirdConfig = secondConfig;
}
secondConfig = config;
} else if (!secondConfig) {
secondConfig = config;
} else if (needThirdFace && !thirdConfig) {
thirdConfig = config;
}
}
CHECK(ownerConfig);This might not be very clear for someone that is not familiar with the FacePile UI, can you add comments and maybe move this in method helper ?
@interface FacePileView : UIView <FacePileConsumer>
@end```suggestion
@interface FacePileView : UIView <FacePileConsumer>
// Designated initializer.
- (instancetype)init NS_DESIGNATED_INITIALIZER;
- (instancetype)initWithFrame:(CGRect)frame NS_UNAVAILABLE;
- (instancetype)initWithCoder:(NSCoder*)aDecoder NS_UNAVAILABLE;
@end
```
NSArray<id<ShareKitAvatarPrimitive>>* _avatars;
UIStackView* _faces;nit: add comments, this is not very clear.
[_faces addArrangedSubview:[avatar view]];Would be nice to add accessibility identifiers to avatar views with their index, could be helpful for testing.
if ([_facePile isDescendantOfView:self]) {I'm curious, is this approach better than `_stackView == _facePile.superview` ?
@class TabGroupsPanelCell;
@class TabGroupsPanelItem;
@class TabGroupsPanelItemData;
@protocol FacePileProviding;```suggestion
@protocol FacePileProviding;
@class TabGroupsPanelCell;
@class TabGroupsPanelItem;
@class TabGroupsPanelItemData;
```
ShareKitFacePileConfiguration* config =you can remove its import
Related details
- Aliona Dangla
- Gauthier Ambard
- Louis Romero
Code-Coverage
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Ewann Pellé (Gerrit)
Ewann Pellé added 13 comments![]( "Open in Gerrit")
// to its consumer (typically a FacePileView).Ewann Pelléoptional nit: `FacePileView consumer`
Done
raw_ptr<data_sharing::DataSharingService> _dataSharingService;optional nit: add comments for group of ivars.
e.g
```
@implementation FacePileMediator {
// Services required by the mediator.
raw_ptr<data_sharing::DataSharingService> _dataSharingService;
raw_ptr<ShareKitService> _shareKitService;
raw_ptr<signin::IdentityManager> _identityManager;// Settings specific to this mediator instance.
FacePileConfiguration* _configuration;// Handles observing changes from services.
std::unique_ptr<DataSharingServiceObserverBridge> _dataSharingServiceObserver;
std::unique_ptr<ScopedDataSharingSyncObservation> _scopedDataSharingServiceObservation;
}
```
Done
_configuration = configuration;
_dataSharingService = dataSharingService;
_shareKitService = shareKitService;
_identityManager = identityManager;Some services can be nullptr ?
Done
if (!_consumer || !_dataSharingService->IsGroupDataModelLoaded()) {
return;
}
CoreAccountInfo account =
_identityManager->GetPrimaryAccountInfo(signin::ConsentLevel::kSignin);
if (account.IsEmpty()) {
// No current logged in user.
return;
}
```suggestion
if (!_consumer || !_dataSharingService->IsGroupDataModelLoaded()) {
return;
}
CoreAccountInfo account =
_identityManager->GetPrimaryAccountInfo(signin::ConsentLevel::kSignin);
if (account.IsEmpty()) {
// No current logged in user.
return;
}```
Done
ShareKitAvatarConfiguration* ownerConfig;This might not be very clear for someone that is not familiar with the FacePile UI, can you add comments and maybe move this in method helper ?
Done
```suggestion
@interface FacePileView : UIView <FacePileConsumer>// Designated initializer.
- (instancetype)init NS_DESIGNATED_INITIALIZER;
- (instancetype)initWithFrame:(CGRect)frame NS_UNAVAILABLE;
- (instancetype)initWithCoder:(NSCoder*)aDecoder NS_UNAVAILABLE;
@end
```
Done
NSArray<id<ShareKitAvatarPrimitive>>* _avatars;
UIStackView* _faces;nit: add comments, this is not very clear.
Done
Would be nice to add accessibility identifiers to avatar views with their index, could be helpful for testing.
Done
@class TabGroupsPanelCell;
@class TabGroupsPanelItem;
@class TabGroupsPanelItemData;
@protocol FacePileProviding;```suggestion
@protocol FacePileProviding;
@class TabGroupsPanelCell;
@class TabGroupsPanelItem;
@class TabGroupsPanelItemData;
```
Done
ShareKitFacePileConfiguration* config =you can remove its import
Done
Related details
- Aliona Dangla
- Gauthier Ambard
- Louis Romero
Code-Coverage
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Louis Romero (Gerrit)
Louis Romero added 11 comments![]( "Open in Gerrit")
baseViewController:(UIViewController*)viewControllerYou don't even need the base view controller. Maybe remove it? Does the parent support having a nil base view controller?
// This coordinator is responsible for creating and managing the FacePileView
// and its associated FacePileMediator.
@interface FacePileCoordinator : ChromeCoordinator <FacePileProviding>```suggestion
// This coordinator is responsible for creating and managing a FacePileView
// and its associated FacePileMediator.
// It is the responsibility of the caller to add the face pile to the view
// hierarchy. On the other hand, when this coordinator is stopped, the view is
// automatically removed from the view hierarchy.
@interface FacePileCoordinator : ChromeCoordinator <FacePileProviding>
```
- (void)setShowTextWhenEmpty:(BOOL)showTextWhenEmpty;Minor nit: ```suggestion
- (void)setShowsTextWhenEmpty:(BOOL)showsTextWhenEmpty;
- ```
// same). FacePiles provided by this object will be correct as long as thisMaybe?
```suggestion
// same). FacePiles provided by this object will be up-to-date as long as this
```
But it seems like an implementation detail of one particular provider, so I wonder if this comment belongs here?
@property(nonatomic, strong) UILabel* emptyFacePileLabel;Why isn't it a private ivar?
Also, it never seems created with the current code?
if (!showTextWhenEmpty && !_emptyFacePileLabel) {
return;
}
self.emptyFacePileLabel.hidden = ![self isEmpty];
}This feels complicated.
What about something like:
```suggestion
if (showsTextWhenEmpty) {
[self addEmptyFacePileLabelIfNeeded];
_emptyFacePileLabel.hidden = ![self isEmpty];
} else {
[_emptyFacePileLabel removeFromSuperview];
_emptyFacePileLabel = nil;
}
```
or
```suggestion
if (showsTextWhenEmpty) {
[self addEmptyFacePileLabelIfNeeded];
[self updateEmptyFacePileLabel];
} else {
[_emptyFacePileLabel removeFromSuperview];
_emptyFacePileLabel = nil;
}
```
then you can reuse `updateEmptyFacePileLabel` in `-updateWithViews:totalNumber:`.
- (void)updateWithViews:(NSArray<id<ShareKitAvatarPrimitive>>*)faces
totalNumber:(NSInteger)totalNumber {
_avatars = faces;Should this also update the hidden property of the empty face pile label, in case the array emptiness changed?
[_facesStackView addArrangedSubview:[avatar view]];When are the old views removed from the stack view?
[_stackView addArrangedSubview:facePile];Did we try having the internal face pile not be in the stack view? Would it have fixed the layout?
Why don't we add the chromium face pile to the stack view also?
_facePileProvider = facePileProvider;Why do you need the provider? Can't the caller just set the face pile externally?
Edit: oh, it's just to keep it in memory. I think this should be better explained in the interface comment. It's rather unusual, no? We have N cells holding onto N coordinators basically (even though they don't know they are coordinators). Coordinators should be part of the coordinator hierarchy instead.
But maybe it's a pattern we already follow elsewhere?
Related details
- Aliona Dangla
- Ewann Pellé
- Gauthier Ambard
Code-Coverage
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Ewann Pellé (Gerrit)
Ewann Pellé added 9 comments![]( "Open in Gerrit")
baseViewController:(UIViewController*)viewControllerYou don't even need the base view controller. Maybe remove it? Does the parent support having a nil base view controller?
Done
// This coordinator is responsible for creating and managing the FacePileView
// and its associated FacePileMediator.
@interface FacePileCoordinator : ChromeCoordinator <FacePileProviding>```suggestion
// This coordinator is responsible for creating and managing a FacePileView
// and its associated FacePileMediator.
// It is the responsibility of the caller to add the face pile to the view
// hierarchy. On the other hand, when this coordinator is stopped, the view is
// automatically removed from the view hierarchy.
@interface FacePileCoordinator : ChromeCoordinator <FacePileProviding>
```
Done
- (void)setShowTextWhenEmpty:(BOOL)showTextWhenEmpty;Minor nit: ```suggestion
- (void)setShowsTextWhenEmpty:(BOOL)showsTextWhenEmpty;
- ```
Done
// same). FacePiles provided by this object will be correct as long as thisMaybe?
```suggestion
// same). FacePiles provided by this object will be up-to-date as long as this
```But it seems like an implementation detail of one particular provider, so I wonder if this comment belongs here?
Done
@property(nonatomic, strong) UILabel* emptyFacePileLabel;Why isn't it a private ivar?
Also, it never seems created with the current code?
Done
if (!showTextWhenEmpty && !_emptyFacePileLabel) {
return;
}
self.emptyFacePileLabel.hidden = ![self isEmpty];
}This feels complicated.
What about something like:
```suggestion
if (showsTextWhenEmpty) {
[self addEmptyFacePileLabelIfNeeded];
_emptyFacePileLabel.hidden = ![self isEmpty];
} else {
[_emptyFacePileLabel removeFromSuperview];
_emptyFacePileLabel = nil;
}
```or
```suggestion
if (showsTextWhenEmpty) {
[self addEmptyFacePileLabelIfNeeded];
[self updateEmptyFacePileLabel];
} else {
[_emptyFacePileLabel removeFromSuperview];
_emptyFacePileLabel = nil;
}
```then you can reuse `updateEmptyFacePileLabel` in `-updateWithViews:totalNumber:`.
Done
- (void)updateWithViews:(NSArray<id<ShareKitAvatarPrimitive>>*)faces
totalNumber:(NSInteger)totalNumber {
_avatars = faces;Should this also update the hidden property of the empty face pile label, in case the array emptiness changed?
Done
[_facesStackView addArrangedSubview:[avatar view]];When are the old views removed from the stack view?
Done
_facePileProvider = facePileProvider;Why do you need the provider? Can't the caller just set the face pile externally?
Edit: oh, it's just to keep it in memory. I think this should be better explained in the interface comment. It's rather unusual, no? We have N cells holding onto N coordinators basically (even though they don't know they are coordinators). Coordinators should be part of the coordinator hierarchy instead.
But maybe it's a pattern we already follow elsewhere?
- Aliona Dangla
- Gauthier Ambard
- Louis Romero
Code-Coverage
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Gauthier Ambard (Gerrit)
Gauthier Ambard added 2 comments![]( "Open in Gerrit")
[_stackView addArrangedSubview:facePile];Did we try having the internal face pile not be in the stack view? Would it have fixed the layout?
Why don't we add the chromium face pile to the stack view also?
For the same reason that it is not working with long name today: the facepile's stack view doesn't have a correct intrinsic size, so adding it in a stack view was doing weird things.
if ([_facePile isDescendantOfView:self]) {I'm curious, is this approach better than `_stackView == _facePile.superview` ?
If we add the facepile as subview of something else than the stack view (as I did below), it is still true.
Related details
- Aliona Dangla
- Ewann Pellé
- Louis Romero
Code-Coverage
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Louis Romero (Gerrit)
Louis Romero added 1 comment![]( "Open in Gerrit")
[_stackView addArrangedSubview:facePile];Gauthier AmbardDid we try having the internal face pile not be in the stack view? Would it have fixed the layout?
Why don't we add the chromium face pile to the stack view also?
For the same reason that it is not working with long name today: the facepile's stack view doesn't have a correct intrinsic size, so adding it in a stack view was doing weird things.
- Aliona Dangla
- Ewann Pellé
- Gauthier Ambard
Code-Coverage
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Louis Romero (Gerrit)
Louis Romero added 2 comments![]( "Open in Gerrit")
CHECK(_facePileView);Maybe add an error log asking to first start the coordinator?
```suggestion
CHECK(_facePileView)
<< "Call -[FacePileCoordinator start] before requesting `facePileView`.";
```
(I somehow miss the AI-suggested fixes already.)
_facePileProvider = facePileProvider;Ewann PelléWhy do you need the provider? Can't the caller just set the face pile externally?
Edit: oh, it's just to keep it in memory. I think this should be better explained in the interface comment. It's rather unusual, no? We have N cells holding onto N coordinators basically (even though they don't know they are coordinators). Coordinators should be part of the coordinator hierarchy instead.
But maybe it's a pattern we already follow elsewhere?
Done
I don't see this addressed?
Ewann Pellé (Gerrit)
Ewann Pellé added 1 comment![]( "Open in Gerrit")
- Aliona Dangla
- Gauthier Ambard
Code-Coverage
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Ewann Pellé (Gerrit)
Ewann Pellé added 4 comments![]( "Open in Gerrit")
CHECK(_facePileView);Maybe add an error log asking to first start the coordinator?
```suggestion
CHECK(_facePileView)
<< "Call -[FacePileCoordinator start] before requesting `facePileView`.";
```(I somehow miss the AI-suggested fixes already.)
Done
[_stackView addArrangedSubview:facePile];Gauthier AmbardDid we try having the internal face pile not be in the stack view? Would it have fixed the layout?
Why don't we add the chromium face pile to the stack view also?
Louis RomeroFor the same reason that it is not working with long name today: the facepile's stack view doesn't have a correct intrinsic size, so adding it in a stack view was doing weird things.
So should we separate that in its own CL?
Acknowledged
_facePileProvider = facePileProvider;Ewann PelléWhy do you need the provider? Can't the caller just set the face pile externally?
Edit: oh, it's just to keep it in memory. I think this should be better explained in the interface comment. It's rather unusual, no? We have N cells holding onto N coordinators basically (even though they don't know they are coordinators). Coordinators should be part of the coordinator hierarchy instead.
But maybe it's a pattern we already follow elsewhere?
Louis RomeroDone
I don't see this addressed?
Added a comment.
if ([_facePile isDescendantOfView:self]) {Gauthier AmbardI'm curious, is this approach better than `_stackView == _facePile.superview` ?
If we add the facepile as subview of something else than the stack view (as I did below), it is still true.
- Aliona Dangla
- Gauthier Ambard
- Louis Romero
Code-Coverage
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Gauthier Ambard (Gerrit)
Gauthier Ambard abandoned this change![]( "Open in Gerrit")
Related details
Code-Coverage
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Gauthier Ambard (Gerrit)
Gauthier Ambard added 3 comments![]( "Open in Gerrit")
totalNumber:(NSInteger)totalNumber;Nit: comments
#import "ui/base/l10n/l10n_util.h"Nit: weird empty lines in this file (one too many at the top too).
@property(nonatomic, strong) id<FacePileProviding> facePileProvider;We are passing it to be kept alive right? Maybe it should be mentioned somewhere?
Related details
- Ewann Pellé
Code-Coverage
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Ewann Pellé (Gerrit)
Ewann Pellé voted and added 3 comments![]( "Open in Gerrit")
Votes added by Ewann Pellé
| Commit-Queue | +1 |
3 comments
totalNumber:(NSInteger)totalNumber;Ewann PelléNit: comments
Done
Nit: weird empty lines in this file (one too many at the top too).
Done
@property(nonatomic, strong) id<FacePileProviding> facePileProvider;We are passing it to be kept alive right? Maybe it should be mentioned somewhere?
- Gauthier Ambard
Code-Coverage
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Ewann Pellé (Gerrit)
Ewann Pellé added 1 comment![]( "Open in Gerrit")
"+components/saved_tab_groups/test_support",will reformat.
Related details
- Gauthier Ambard
Code-Coverage
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Gauthier Ambard (Gerrit)
Gauthier Ambard voted and added 5 comments![]( "Open in Gerrit")
Votes added by Gauthier Ambard
| Code-Review | +1 |
5 comments
[super stop];Here and above, I think the coordinators should not call super on start/stop.
- (void)updateWithViews:(NSArray<id<ShareKitAvatarPrimitive>>*)facesShould this be "views" or "faces"?
// Updates the FacePileView with a new set of avatar primitives and the totalNit: update comments
// found in the LICENSE file.Nit: empty line
#pragma mark - FacePileConsumerNit: add empty line
Related details
- Ewann Pellé
Code-Coverage
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Ewann Pellé (Gerrit)
Ewann Pellé voted and added 6 comments![]( "Open in Gerrit")
Votes added by Ewann Pellé
| Commit-Queue | +2 |
6 comments
"+components/saved_tab_groups/test_support",Ewann Pelléwill reformat.
Done
Here and above, I think the coordinators should not call super on start/stop.
Done
- (void)updateWithViews:(NSArray<id<ShareKitAvatarPrimitive>>*)facesShould this be "views" or "faces"?
Done
// Updates the FacePileView with a new set of avatar primitives and the totalEwann PelléNit: update comments
I guess you prefer `faces` ?
// found in the LICENSE file.Ewann PelléNit: empty line
Done
#pragma mark - FacePileConsumerEwann PelléNit: add empty line
Done
Related details
Code-Coverage
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Ewann Pellé (Gerrit)
Ewann Pellé voted Commit-Queue+0![]( "Open in Gerrit")
| Commit-Queue | +0 |
Related details
Code-Coverage
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Ewann Pellé (Gerrit)
Ewann Pellé voted Commit-Queue+2![]( "Open in Gerrit")
| Commit-Queue | +2 |
Related details
Code-Coverage
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Chromium LUCI CQ (Gerrit)
Chromium LUCI CQ submitted the change with unreviewed changes![]( "Open in Gerrit")
Unreviewed changes
4 is the latest approved patch-set.
The change was submitted with unreviewed changes in the following files:
```
The name of the file: ios/chrome/browser/saved_tab_groups/ui/face_pile_view.mm
Insertions: 3, Deletions: 1.
The diff is too large to show. Please review the diff.
```
```
The name of the file: ios/chrome/browser/saved_tab_groups/ui/fake_face_pile_consumer.mm
Insertions: 1, Deletions: 1.
The diff is too large to show. Please review the diff.
```
```
The name of the file: ios/chrome/browser/saved_tab_groups/coordinator/face_pile_mediator.mm
Insertions: 1, Deletions: 1.
The diff is too large to show. Please review the diff.
```
```
The name of the file: ios/chrome/browser/saved_tab_groups/ui/face_pile_consumer.h
Insertions: 3, Deletions: 3.
The diff is too large to show. Please review the diff.
```
```
The name of the file: ios/chrome/browser/saved_tab_groups/coordinator/face_pile_coordinator.mm
Insertions: 0, Deletions: 3.
The diff is too large to show. Please review the diff.
```
```
The name of the file: ios/chrome/browser/saved_tab_groups/coordinator/DEPS
Insertions: 2, Deletions: 2.
The diff is too large to show. Please review the diff.
```
Change information
[iOS] Create a facepile implementation in Chrome
As the one from downstream has issues, this a skeleton of what it would
look like in Chromium.
Based on: https://crrev.com/c/6596982
- A ios/chrome/browser/saved_tab_groups/coordinator/BUILD.gn
- A ios/chrome/browser/saved_tab_groups/coordinator/DEPS
- A ios/chrome/browser/saved_tab_groups/coordinator/face_pile_configuration.h
- A ios/chrome/browser/saved_tab_groups/coordinator/face_pile_configuration.mm
- A ios/chrome/browser/saved_tab_groups/coordinator/face_pile_coordinator.h
- A ios/chrome/browser/saved_tab_groups/coordinator/face_pile_coordinator.mm
- A ios/chrome/browser/saved_tab_groups/coordinator/face_pile_mediator.h
- A ios/chrome/browser/saved_tab_groups/coordinator/face_pile_mediator.mm
- A ios/chrome/browser/saved_tab_groups/coordinator/face_pile_mediator_unittest.mm
- M ios/chrome/browser/saved_tab_groups/ui/BUILD.gn
- A ios/chrome/browser/saved_tab_groups/ui/DEPS
- A ios/chrome/browser/saved_tab_groups/ui/face_pile_consumer.h
- A ios/chrome/browser/saved_tab_groups/ui/face_pile_providing.h
- A ios/chrome/browser/saved_tab_groups/ui/face_pile_view.h
- A ios/chrome/browser/saved_tab_groups/ui/face_pile_view.mm
- A ios/chrome/browser/saved_tab_groups/ui/fake_face_pile_consumer.h
- A ios/chrome/browser/saved_tab_groups/ui/fake_face_pile_consumer.mm
- A ios/chrome/browser/saved_tab_groups/ui/fake_face_pile_provider.h
- A ios/chrome/browser/saved_tab_groups/ui/fake_face_pile_provider.mm
- M ios/chrome/browser/tab_switcher/ui_bundled/tab_grid/grid/regular/BUILD.gn
- M ios/chrome/browser/tab_switcher/ui_bundled/tab_grid/tab_groups/BUILD.gn
- M ios/chrome/browser/tab_switcher/ui_bundled/tab_grid/tab_groups/DEPS
- M ios/chrome/browser/tab_switcher/ui_bundled/tab_grid/tab_groups/tab_groups_panel_cell.h
- M ios/chrome/browser/tab_switcher/ui_bundled/tab_grid/tab_groups/tab_groups_panel_cell.mm
- M ios/chrome/browser/tab_switcher/ui_bundled/tab_grid/tab_groups/tab_groups_panel_coordinator.mm
- M ios/chrome/browser/tab_switcher/ui_bundled/tab_grid/tab_groups/tab_groups_panel_item_data_source.h
- M ios/chrome/browser/tab_switcher/ui_bundled/tab_grid/tab_groups/tab_groups_panel_mediator.mm
- M ios/chrome/browser/tab_switcher/ui_bundled/tab_grid/tab_groups/tab_groups_panel_mediator_delegate.h
- M ios/chrome/browser/tab_switcher/ui_bundled/tab_grid/tab_groups/tab_groups_panel_mediator_unittest.mm
- M ios/chrome/browser/tab_switcher/ui_bundled/tab_grid/tab_groups/tab_groups_panel_view_controller.mm
- M ios/chrome/test/BUILD.gn
Code-Review: +1 by Gauthier Ambard
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |