[Adium-devl] [PATCH] CL filtering expanding fix (want review)

[Colin Barrett]

Zac reviewed this via IM, I pushed it earlier today.

http://hg.adiumx.com/adium/rev/7c54b52a482a

-Colin

On Apr 24, 2009, at 12:52 AM, Colin Barrett wrote:

> I've been seeing a number of problems with CL filtering causing  
> groups to expand and contract. I have what I think is a pretty good  
> patch to fix this issue. Quoting patch's commit message:
>
>> Fix many problems with the contact list filtering not properly  
>> restoring group expanding, and not properly expanding groups.
>>
>> Instead of trying to twiddle the view proxies directly, we set a  
>> special display property on each expanded group's model object. The  
>> list controller checks this property when deciding if an item is  
>> expanded or not.
>
> I have another patch that fixes this in a way more similar to the  
> old method of poking the state of the model objects directly (but  
> this time saving things properly by stashing the old values in,  
> again, display properties), but this way seems cleaner to me:  
> expanding during filtering is a temporary view expansion and  
> shouldn't actually change the state of the object. Additionally,  
> actually flipping the model object's state means that if we crash  
> during a filtering operation, there's a chance user's groups will  
> all be expanded when she starts up next.
>
> What do others think of this? I've copy pasted the patch below, and  
> attached a bundle. Personally, I'm a little wary of [contactListView  
> reloadData], it seems too straightforwad for Adium code ;)
>
> Suggestions welcome!
>
> <expandfix.hgbundle>
>
> =====================================================================
>
> diff --git a/Frameworks/Adium Framework/Source/ 
> AIAbstractListController.m b/Frameworks/Adium Framework/Source/ 
> AIAbstractListController.m
> --- a/Frameworks/Adium Framework/Source/AIAbstractListController.m	
> +++ b/Frameworks/Adium Framework/Source/AIAbstractListController.m	
> @@ -673,7 +673,7 @@
>  //
>  - (BOOL)outlineView:(NSOutlineView *)outlineView expandStateOfItem: 
> (AIProxyListObject *)item
>  {
> -    return !item || ((id<AIContainingObject>) 
> (item.listObject)).expanded;
> +    return !item || ((id<AIContainingObject>) 
> (item.listObject)).expanded || [item.listObject  
> boolValueForProperty:@"ExpandedByFiltering"];
>  }
>
>  /*!
> diff --git a/Source/AIListWindowController.m b/Source/ 
> AIListWindowController.m
> --- a/Source/AIListWindowController.m
> +++ b/Source/AIListWindowController.m
> @@ -1666,54 +1666,34 @@
>  		return;
>  	
>  	if (!filterBarExpandedGroups && ![[sender stringValue]  
> isEqualToString:@""]) {
> -		// Temporarily expand all groups when performing a search.
> -		NSMutableArray *groupsToExpand = [NSMutableArray array];
> -		NSUInteger rows = [contactListView numberOfRows];
> -		for (int i = 0; i < rows; i++) {
> -			AIProxyListObject *proxyObject = [contactListView itemAtRow:i];
> -			if ([proxyObject isKindOfClass:[AIListGroup class]] &&  
> ((AIListGroup *)proxyObject).expanded == NO)
> -				[groupsToExpand addObject:proxyObject];
> -		}
> -		
> -		if (groupsToExpand.count) {
> -			for (AIProxyListObject *proxyObject in groupsToExpand) {
> -				// Force the listgroup to save its expanded status
> -				AIListGroup *proxiedObject = (AIListGroup *)proxyObject;
> -				
> -				[proxiedObject setPreference:[NSNumber  
> numberWithBool:proxiedObject.expanded]
> -									  forKey:@"IsExpanded"
> -									   group:@"Contact List"];
> -				
> -				// Set the group as expanded
> -				[contactListView expandItem:proxyObject];
> -			}			
> -		}
> -		
> -		filterBarExpandedGroups = YES;
> -		
> -	} else if (filterBarExpandedGroups && [[sender stringValue]  
> isEqualToString:@""]) {
> -		// Restore saved expansion status when returning to no search.
> -		
> -		// Temporarily expand all groups when performing a search.
> -		NSMutableArray *groupsToCollapse = [NSMutableArray array];
> -		NSUInteger rows = [contactListView numberOfRows];
> -		for (int i = 0; i < rows; i++) {
> -			AIProxyListObject *proxyObject = [contactListView itemAtRow:i];
> -			
> -			if ([proxyObject isKindOfClass:[AIListGroup class]] &&
> -				![[(AIListGroup *)proxyObject preferenceForKey:@"IsExpanded"  
> group:@"Contact List"] boolValue]) {
> -				[groupsToCollapse addObject:proxyObject];
> -			}
> -		}
> -		
> -		if (groupsToCollapse.count) {
> -			for (AIProxyListObject *proxyObject in groupsToCollapse) {
> -				[contactListView collapseItem:proxyObject];
> -			}			
> -		}
> -		
> -		filterBarExpandedGroups = NO;
> -	}
> +        BOOL modified = NO;
> +        for (AIListObject *listObject in [self.contactList  
> containedObjects]) {
> +            if ([listObject isKindOfClass:[AIListGroup class]] &&  
> [(AIListGroup *)listObject isExpanded] == NO) {
> +                [listObject setValue:[NSNumber numberWithBool:YES]  
> forProperty:@"ExpandedByFiltering" notify:NotifyNever];
> +                modified = YES;
> +            }
> +        }
> +
> +        filterBarExpandedGroups = YES;
> +
> +        if (modified) {
> +            [contactListView reloadData];
> +        }
> +    } else if (filterBarExpandedGroups && [[sender stringValue]  
> isEqualToString:@""]) {
> +        BOOL modified = NO;
> +        for (AIListObject *listObject in [self.contactList  
> containedObjects]) {
> +            if ([listObject isKindOfClass:[AIListGroup class]] &&  
> [listObject boolValueForProperty:@"ExpandedByFiltering"]) {
> +                [listObject setValue:[NSNumber numberWithBool:NO]  
> forProperty:@"ExpandedByFiltering" notify:NotifyNever];
> +                modified = YES;
> +            }
> +        }
> +
> +        filterBarExpandedGroups = NO;
> +
> +        if (modified) {
> +            [contactListView reloadData];
> +        }
> +    }
>  	
>  	if ([[AIContactHidingController sharedController] filterContacts: 
> [sender stringValue]]) {
>  		// Select the first contact; we're guaranteed at least one  
> visible contact.
>
> _______________________________________________
> Adium-devl mailing list
> Adium-devl at adiumx.com
> http://adiumx.com/mailman/listinfo/adium-devl_adiumx.com

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://adium.im/pipermail/devel_adium.im/attachments/20090424/09e967ce/attachment-0001.html>