Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove experimental features (exp_skip_a11y_wait && exp_new_default_cell_layout_mode) #1383

Merged
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions Schemas/configuration.json
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,6 @@
"exp_skip_clear_data",
"exp_did_enter_preload_skip_asm_layout",
"exp_disable_a11y_cache",
"exp_skip_a11y_wait",
"exp_new_default_cell_layout_mode",
"exp_dispatch_apply"
]
}
Expand Down
11 changes: 2 additions & 9 deletions Source/ASCollectionView.mm
Original file line number Diff line number Diff line change
Expand Up @@ -317,12 +317,7 @@ - (instancetype)_initWithFrame:(CGRect)frame collectionViewLayout:(UICollectionV

[self _configureCollectionViewLayout:layout];

if (ASActivateExperimentalFeature(ASExperimentalNewDefaultCellLayoutMode)) {
_cellLayoutMode = ASCellLayoutModeSyncForSmallContent;
} else {
_cellLayoutMode = ASCellLayoutModeNone;
}

_cellLayoutMode = ASCellLayoutModeSyncForSmallContent;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one can go away as the default one will be ASCellLayoutModeNone. In fact the ASCellLayoutModeSyncForSmallContent is not needed at all anymore

return self;
}

Expand Down Expand Up @@ -2487,9 +2482,7 @@ - (void)setPrefetchingEnabled:(BOOL)prefetchingEnabled

- (NSArray *)accessibilityElements
{
if (!ASActivateExperimentalFeature(ASExperimentalSkipAccessibilityWait)) {
[self waitUntilAllUpdatesAreCommitted];
}
[self waitUntilAllUpdatesAreCommitted];
return [super accessibilityElements];
}

Expand Down
2 changes: 0 additions & 2 deletions Source/ASExperimentalFeatures.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,6 @@ typedef NS_OPTIONS(NSUInteger, ASExperimentalFeatures) {
ASExperimentalSkipClearData = 1 << 7, // exp_skip_clear_data
ASExperimentalDidEnterPreloadSkipASMLayout = 1 << 8, // exp_did_enter_preload_skip_asm_layout
ASExperimentalDisableAccessibilityCache = 1 << 9, // exp_disable_a11y_cache
ASExperimentalSkipAccessibilityWait = 1 << 10, // exp_skip_a11y_wait
ASExperimentalNewDefaultCellLayoutMode = 1 << 11, // exp_new_default_cell_layout_mode
ASExperimentalDispatchApply = 1 << 12, // exp_dispatch_apply
ASExperimentalFeatureAll = 0xFFFFFFFF
};
Expand Down
2 changes: 0 additions & 2 deletions Source/ASExperimentalFeatures.mm
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,6 @@
@"exp_skip_clear_data",
@"exp_did_enter_preload_skip_asm_layout",
@"exp_disable_a11y_cache",
@"exp_skip_a11y_wait",
@"exp_new_default_cell_layout_mode",
@"exp_dispatch_apply"]));

if (flags == ASExperimentalFeatureAll) {
Expand Down
30 changes: 13 additions & 17 deletions Source/ASTableView.mm
Original file line number Diff line number Diff line change
Expand Up @@ -1678,20 +1678,18 @@ - (BOOL)dataControllerShouldSerializeNodeCreation:(ASDataController *)dataContro

- (BOOL)dataController:(ASDataController *)dataController shouldSynchronouslyProcessChangeSet:(_ASHierarchyChangeSet *)changeSet
{
if (ASActivateExperimentalFeature(ASExperimentalNewDefaultCellLayoutMode)) {
// Reload data is expensive, don't block main while doing so.
if (changeSet.includesReloadData) {
Copy link
Contributor

@maicki maicki Mar 7, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason we removed this if clause?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code was not there when the new layout changes was introduced. This is added when we created the experiment afterward. I checked with @nguyenhuy that we did not experiment without this block of code. I will put it back.

return NO;
}
// For more details on this method, see the comment in the ASCollectionView implementation.
if (changeSet.countForAsyncLayout < 2) {
return YES;
}
CGSize contentSize = self.contentSize;
CGSize boundsSize = self.bounds.size;
if (contentSize.height <= boundsSize.height && contentSize.width <= boundsSize.width) {
return YES;
}
// Reload data is expensive, don't block main while doing so.
if (changeSet.includesReloadData) {
return NO;
}
// For more details on this method, see the comment in the ASCollectionView implementation.
if (changeSet.countForAsyncLayout < 2) {
return YES;
}
CGSize contentSize = self.contentSize;
CGSize boundsSize = self.bounds.size;
if (contentSize.height <= boundsSize.height && contentSize.width <= boundsSize.width) {
return YES;
}
return NO;
}
Expand Down Expand Up @@ -2011,9 +2009,7 @@ - (void)didMoveToSuperview

- (NSArray *)accessibilityElements
{
if (!ASActivateExperimentalFeature(ASExperimentalSkipAccessibilityWait)) {
[self waitUntilAllUpdatesAreCommitted];
}
[self waitUntilAllUpdatesAreCommitted];
return [super accessibilityElements];
}

Expand Down
4 changes: 0 additions & 4 deletions Tests/ASConfigurationTests.mm
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,6 @@
ASExperimentalSkipClearData,
ASExperimentalDidEnterPreloadSkipASMLayout,
ASExperimentalDisableAccessibilityCache,
ASExperimentalSkipAccessibilityWait,
ASExperimentalNewDefaultCellLayoutMode,
ASExperimentalDispatchApply
};

Expand All @@ -53,8 +51,6 @@ + (NSArray *)names {
@"exp_skip_clear_data",
@"exp_did_enter_preload_skip_asm_layout",
@"exp_disable_a11y_cache",
@"exp_skip_a11y_wait",
@"exp_new_default_cell_layout_mode",
@"exp_dispatch_apply"
];
}
Expand Down