-1

In my collection view when (custom) cells are reused they, again, get the highlight I have set in didSelectItemAtIndexPath for the original selection. To prevent this, I am using the custom cell's prepareForReuse method, and post calling [super], I check to see if its the selected cell.

If it is I am change the highlight to default else I restore to the original selection highlight when the cell in question is brought back in scroll view's visible area.

Here's the code...

- (void)prepareForReuse
{
    [super prepareForReuse];

    if (!self.isSelected) {
        [self setBackgroundColor:[UIColor systemBackgroundColor]];
        [_tagImageView setTintColor:[UIColor systemBlueColor]];
    }
    else if (self.isSelected)
    {
        [self setBackgroundColor:[UIColor systemBlueColor]];
        [_tagImageView setTintColor:[UIColor systemBackgroundColor]];
    }

}

But I notice that the second if block is never executed even when I bring back the original cell in view. This is where I need help. How do I ensure re-highlighting or the original cell/item?

Note, if I try and save the original cell- even though not highlighted, remains the one selected and the corresponding value is saved. So, this is just about the re-highlight.

Also, here is the selection code...* didSelectItemAtIndexPath*

- (void)collectionView:(UICollectionView *)collectionView
didSelectItemAtIndexPath:(NSIndexPath *)indexPath
{
    if (selectedIndexPath!=nil) {
        if (indexPath.row==selectedIndexPath.row)
        {
            [tagCollectionView deselectItemAtIndexPath:indexPath animated:YES];
            TagCollectionViewCell *selectedCell =  (TagCollectionViewCell *)[tagCollectionView cellForItemAtIndexPath:selectedIndexPath];
            selectedCell.backgroundColor = [UIColor clearColor];
            selectedCell.tagImageView.tintColor = [UIColor systemBlueColor];
            selectedIndexPath=nil;
            [newDictionary setValue:[NSNull null] forKey:@"type"];
        }
        else
        {
            [tagCollectionView deselectItemAtIndexPath:indexPath animated:YES];
            TagCollectionViewCell *previousSelectedCell =  (TagCollectionViewCell *)[tagCollectionView cellForItemAtIndexPath:selectedIndexPath];
            previousSelectedCell.backgroundColor = [UIColor systemBackgroundColor];
            previousSelectedCell.tagImageView.tintColor = [UIColor systemBlueColor];
            selectedIndexPath = indexPath;
            TagCollectionViewCell *selectedCell =  (TagCollectionViewCell *)[tagCollectionView cellForItemAtIndexPath:selectedIndexPath];
            selectedCell.backgroundColor = [UIColor systemBlueColor];
            selectedCell.tagImageView.tintColor = [UIColor systemBackgroundColor];
            dictionaryType = _typesArray[selectedIndexPath.row];
            [newDictionary setValue:dictionaryType forKey:@"type"];
        }
    }
    else if (selectedIndexPath==nil)
    {
        selectedIndexPath = indexPath;
        TagCollectionViewCell *selectedCell =  (TagCollectionViewCell *)[tagCollectionView cellForItemAtIndexPath:selectedIndexPath];
        selectedCell.backgroundColor = [UIColor systemBlueColor];
        selectedCell.tagImageView.tintColor = [UIColor systemBackgroundColor];
        dictionaryType = _typesArray[selectedIndexPath.row];
        [newDictionary setValue:dictionaryType forKey:@"type"];
    }
}

Any help? Thanks.

Edit: This is the part of the code that doesn't get called.

else if (self.isSelected)
{
    [self setBackgroundColor:[UIColor systemBlueColor]];
    [_tagImageView setTintColor:[UIColor systemBackgroundColor]];
}
AceN
  • 771
  • 10
  • 18

1 Answers1

1

I think you are way over-complicating things.

A UICollectionView keeps track of its own "selected" cell(s), and calls setSelected on each cell when it is displayed.

You can put all of your "selected" appearance code inside your cell class:

- (void)setSelected:(BOOL)selected {
    // change our color properties based on selected BOOL value
    self.tagImageView.tintColor = selected ? UIColor.systemBackgroundColor : UIColor.systemBlueColor;
    self.backgroundColor = selected ? UIColor.systemBlueColor : UIColor.systemBackgroundColor;
}

Now you don't need to do anything in didSelectItemAt.

Here's a quick example...

SampleCollectionViewCell.h

@interface SampleCollectionViewCell : UICollectionViewCell
- (void)fillData:(NSInteger)n;
@end

SampleCollectionViewCell.m

#import "SampleCollectionViewCell.h"

@interface SampleCollectionViewCell ()
{
    UIImageView *theImageView;
    UILabel *theLabel;
}
@end

@implementation SampleCollectionViewCell

- (instancetype)init
{
    self = [super init];
    if (self) {
        [self commonInit];
    }
    return self;
}
- (instancetype)initWithFrame:(CGRect)frame
{
    self = [super initWithFrame:frame];
    if (self) {
        [self commonInit];
    }
    return self;
}
- (instancetype)initWithCoder:(NSCoder *)coder
{
    self = [super initWithCoder:coder];
    if (self) {
        [self commonInit];
    }
    return self;
}

- (void)commonInit {
    
    // add an image view and a label
    
    theImageView = [UIImageView new];
    theImageView.translatesAutoresizingMaskIntoConstraints = NO;
    [self.contentView addSubview:theImageView];
    
    theLabel = [UILabel new];
    theLabel.textAlignment = NSTextAlignmentCenter;
    theLabel.font = [UIFont systemFontOfSize:20.0 weight:UIFontWeightBold];
    theLabel.translatesAutoresizingMaskIntoConstraints = NO;
    [self.contentView addSubview:theLabel];

    [NSLayoutConstraint activateConstraints:@[
        
        [theImageView.topAnchor constraintEqualToAnchor:self.contentView.topAnchor constant:0.0],
        [theImageView.leadingAnchor constraintEqualToAnchor:self.contentView.leadingAnchor constant:0.0],
        [theImageView.trailingAnchor constraintEqualToAnchor:self.contentView.trailingAnchor constant:0.0],
        [theImageView.bottomAnchor constraintEqualToAnchor:self.contentView.bottomAnchor constant:0.0],
        
        [theLabel.leadingAnchor constraintEqualToAnchor:self.contentView.leadingAnchor constant:0.0],
        [theLabel.trailingAnchor constraintEqualToAnchor:self.contentView.trailingAnchor constant:0.0],
        [theLabel.bottomAnchor constraintEqualToAnchor:self.contentView.bottomAnchor constant:-4.0],
        
    ]];

    // image would probably be set by the data source, but
    //  for this example we'll use the same system image in every cell
    UIImage *img = [UIImage systemImageNamed:@"person.fill"];
    if (img) {
        theImageView.image = img;
    }
    
    // let's give the content view rounded corners and a border
    self.contentView.layer.cornerRadius = 8.0;
    self.contentView.layer.borderWidth = 2.0;
    self.contentView.layer.borderColor = UIColor.systemGreenColor.CGColor;

    // default (not-selected) colors
    theImageView.tintColor = UIColor.cyanColor;
    theLabel.textColor = UIColor.blackColor;
    self.contentView.backgroundColor = UIColor.systemBackgroundColor;

}

- (void)fillData:(NSInteger)n {
    theLabel.text = [NSString stringWithFormat:@"%ld", (long)n];
}

- (void)setSelected:(BOOL)selected {
    // change our color properties based on selected BOOL value
    theImageView.tintColor = selected ? UIColor.redColor : UIColor.cyanColor;
    theLabel.textColor = selected ? UIColor.yellowColor : UIColor.blackColor;
    self.contentView.backgroundColor = selected ? UIColor.systemBlueColor : UIColor.systemBackgroundColor;
}

@end

SampleViewController.h

@interface SampleViewController : UIViewController <UICollectionViewDelegate, UICollectionViewDataSource>

@end

SampleViewController.m

#import "SampleViewController.h"
#import "SampleCollectionViewCell.h"

@interface SampleViewController ()
{
    UICollectionView *collectionView;
}
@end

@implementation SampleViewController

- (void)viewDidLoad {
    [super viewDidLoad];

    UICollectionViewFlowLayout *fl = [UICollectionViewFlowLayout new];
    fl.scrollDirection = UICollectionViewScrollDirectionVertical;
    fl.itemSize = CGSizeMake(60, 60);
    fl.minimumLineSpacing = 8;
    fl.minimumInteritemSpacing = 8;
    
    collectionView = [[UICollectionView alloc] initWithFrame:CGRectZero collectionViewLayout:fl];
    collectionView.translatesAutoresizingMaskIntoConstraints = NO;
    [self.view addSubview:collectionView];
    
    UILayoutGuide *g = [self.view safeAreaLayoutGuide];
    
    [NSLayoutConstraint activateConstraints:@[
        
        // constrain collection view 40-points from all 4 sides
        [collectionView.topAnchor constraintEqualToAnchor:g.topAnchor constant:40.0],
        [collectionView.leadingAnchor constraintEqualToAnchor:g.leadingAnchor constant:40.0],
        [collectionView.trailingAnchor constraintEqualToAnchor:g.trailingAnchor constant:-40.0],
        [collectionView.bottomAnchor constraintEqualToAnchor:g.bottomAnchor constant:-40.0],

    ]];

    [collectionView registerClass:SampleCollectionViewCell.class forCellWithReuseIdentifier:@"c"];
    collectionView.dataSource = self;
    collectionView.delegate = self;
    
    // let's give the collection view a very light gray background
    //  so we can see its frame
    collectionView.backgroundColor = [UIColor colorWithWhite:0.95 alpha:1.0];
}

- (NSInteger)collectionView:(UICollectionView *)collectionView numberOfItemsInSection:(NSInteger)section {
    return 50;
}

- (__kindof UICollectionViewCell *)collectionView:(UICollectionView *)collectionView cellForItemAtIndexPath:(NSIndexPath *)indexPath {
    SampleCollectionViewCell *c = (SampleCollectionViewCell *)[collectionView dequeueReusableCellWithReuseIdentifier:@"c" forIndexPath:indexPath];
    [c fillData:indexPath.item];
    return c;
}

@end

Based on the code you posted, it looks like you want to be able to de-select an already selected cell. If so, add this to the controller:

// this allows us to de-select an already selected cell
- (BOOL)collectionView:(UICollectionView *)collectionView shouldSelectItemAtIndexPath:(NSIndexPath *)indexPath {
    
    // get array of already selected index paths
    NSArray *a = [collectionView indexPathsForSelectedItems];

    // if that array contains indexPath, that means
    //  it is already selected, so
    if ([a containsObject:indexPath]) {
        // deselect it
        [collectionView deselectItemAtIndexPath:indexPath animated:NO];
        return NO;
    }
    // no indexPaths (cells) were selected
    return YES;
    
}

When run, it starts like this:

enter image description here

Tapping cell "1" selects it:

enter image description here

Tapping cell "7" automatically de-selects cell "1" and selects cell "7":

enter image description here

We can scroll up and down and the selected cell will automatically maintain its "selected appearance":

enter image description here enter image description here


Edit

To explain why your prepareForReuse wasn't doing what you expected...

The collection view does not set the selected property of the cell until it is going to be displayed.

So, in:

- (void)prepareForReuse
{
    [super prepareForReuse];

    if (!self.isSelected) {
        [self setBackgroundColor:[UIColor systemBackgroundColor]];
        [_tagImageView setTintColor:[UIColor systemBlueColor]];
    }
    else if (self.isSelected)
    {
        [self setBackgroundColor:[UIColor systemBlueColor]];
        [_tagImageView setTintColor:[UIColor systemBackgroundColor]];
    }

}

self.isSelected will never be true.

If you want to stick with changing the cell UI properties (colors, tint, etc) in didSelectItemAt, you need to update your cell appearance in cellForItemAt:

- (__kindof UICollectionViewCell *)collectionView:(UICollectionView *)collectionView cellForItemAtIndexPath:(NSIndexPath *)indexPath {
    TagCollectionViewCell *c = (TagCollectionViewCell *)[collectionView dequeueReusableCellWithReuseIdentifier:@"c" forIndexPath:indexPath];

    // whatever you are currently doing, such as
    //c.tagImageView.image = ...;
    
    if (selectedIndexPath != indexPath) {
        [c setBackgroundColor:[UIColor systemBackgroundColor]];
        [c.tagImageView setTintColor:[UIColor systemBlueColor]];
    }
    else
    {
        [c setBackgroundColor:[UIColor systemBlueColor]];
        [c.tagImageView setTintColor:[UIColor systemBackgroundColor]];
    }
    
    return c;
}
DonMag
  • 69,424
  • 5
  • 50
  • 86
  • This doesn't solve the original problem where a cell is reselected (ie, shows custom highlight) when its reused. It is this reason that it is suggested elsewhere to handle this in prepareForReuse. For example, above 7 is selected. Upon scrolling ahead you might noticed that even 35 is selected, incase its the same cell as that in 7. So, in prepare for reuse, effort is made to set it back to original unselected highlighting. Now when its reused to show the original cell 7, it should rehighlight the cell. Thats what I'm trying to do in the second 'if' of prepareForReuse. – AceN Jul 21 '22 at 05:53
  • @AceN - did you try running that sample code? I'm guessing your answer is "No" -- because if you had, you would see that this ***explicitly solves*** your cell reuse problem. In your original code, you are changing properties of a **cell** (and its subviews). Taking the approach I offered, we let the **collection view** manage the cell selection / de-selection, and the cell appearance. Change the number of items in the sample code to 100, 500, 1000 ... select a cell, do some scrolling, select another cell, do some scrolling... you'll see that the problem is gone. – DonMag Jul 21 '22 at 12:14
  • @AceN - see the **Edit** to my answer for an explanation of why `prepareForReuse` doesn't do what you expect. – DonMag Jul 21 '22 at 13:08
  • Yes I tried your code for setSelected and also put under comments my didSelect delegate call. What I am trying to say is it brings back the original problem that the cell gets reselected when reused! – AceN Jul 21 '22 at 19:21
  • @AceN - that's confusing... when you tried my code **as-is** ... did you see that the problem is not there? And then you added back in *your* code and the problem returns? – DonMag Jul 21 '22 at 19:59
  • As I said, I tried your code for setSelected. The rest of the code doesn’t apply to me. – AceN Jul 22 '22 at 12:26
  • @AceN - *"The rest of the code doesn’t apply to me"* ... well, I guess that's true if you don't want to solve your issue. Did you look at the **Edit** in my answer that explains what **your code** is doing wrong? (where I also included a fix if you want to stick with your original approach) – DonMag Jul 22 '22 at 12:31
  • @AceN - I put an entire project up on GitHub... shows both approaches and the solutions to the cell reuse problem. https://github.com/DonMag/AvoidCellReuseProblem – DonMag Jul 22 '22 at 13:56
  • thanks a lot! Im travelling atm and will implement it once I get home tomorrow. – AceN Jul 23 '22 at 17:27
  • Your first solution works perfectly. Thank you so much for your time. I looked at the second solution as well but couldn't understand it. I couldn't understand when you said "The collection view does not set the selected property of the cell until it is going to be displayed." As, even without the selection highlight if I were to save, the entry gets saved with the tag that was originally selected. I things, once selected - unless deselected or change in selection, the cell remains selected nevertheless. – AceN Jul 24 '22 at 06:47
  • Once selected - unless deselected or change in selection, the cell remains selected nevertheless. – – AceN Jul 24 '22 at 16:08