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

Support for multiple selections #126

Closed
wants to merge 3 commits into from

Conversation

WanderingStar
Copy link

Added a new flag enabling multiple selections and replaced _selectedIndex with an NSIndexSet

@evadne
Copy link
Collaborator

evadne commented Jul 23, 2012

Currently looking at this pull request. A few points that I’d like to bounce:

  • It seems like tapping on a selected cell does not toggle it — cell always stays selected.
  • It might be better to use allowsMultipleSelection as it is more closer to NSCollectionView terminology.

Aside from that, great patch, merged already and doing cleanup. Thanks. :)

@WanderingStar
Copy link
Author

Hmm. That's odd about the toggling. I'm definitely using that feature in the code that I forked this for. Did you find the problem already, or should I have a look at it?

That naming change sounds good.

@evadne
Copy link
Collaborator

evadne commented Jul 23, 2012

Changes are now on the develop branch. :)

On Jul 23, 2012, at 21:57, Aneel Nazareth
reply@reply.github.com
wrote:

Hmm. That's odd about the toggling. I'm definitely using that feature in the code that I forked this for. Did you find the problem already, or should I have a look at it?

That naming change sounds good.


Reply to this email directly or view it on GitHub:
#126 (comment)

@diegomagal
Copy link

Hi,

I tried to use this code on the develop branch but it do not seem to be alreayd added. Could you check it please?

@evadne
Copy link
Collaborator

evadne commented Aug 15, 2012

This feature is not supported yet. You might want to look into alternate
solutions in the mean time. Sorry for the inconvenience. :)

On Aug 15, 2012, at 6:03, diegomagal notifications@github.com wrote:

Hi,

I tried to use this code on the develop branch but it do not seem to be
alreayd added. Could you check it please?


Reply to this email directly or view it on
GitHubhttps://github.com//pull/126#issuecomment-7755185.

@fbayle
Copy link

fbayle commented Aug 21, 2012

Hi !

This AQGridView is a very nice framework, and I'm already toying with the dev version, as I need multiple selection.

It occurred to me that when setAllowsMultipleSelection is called with false, you should unselect the selected items or, at least, add a unselectAll method.

For now, here is my quick and dirty way to address that :

- (void) setAllowsMultipleSelection:(BOOL)allowsMultipleSelection 
{
    _flags.allowsMultipleSelection = (allowsMultipleSelection ? 1 : 0);
    //  We should unselect the items...
    if (!allowsMultipleSelection)
    {
        if ([self.selectionIndexes count]>0)
        {
            [self beginUpdates];
            int currentIndex=[self.selectionIndexes firstIndex];
            while (currentIndex!=NSNotFound)
            {
                [self deselectItemAtIndex:currentIndex animated:YES];
                currentIndex=[self.selectionIndexes indexGreaterThanIndex:currentIndex];
            }
            [self endUpdates];
        }
    }
}

Frederic

@evadne
Copy link
Collaborator

evadne commented Aug 21, 2012

@fbayle I think you should be able to deselect things by setting
selectionIndexes to nil. Will check shortly.

On Aug 21, 2012, at 6:46, fbayle notifications@github.com wrote:

Hi !

This AQGridView is a very nice framework, and I'm already toying with the
dev version, as I need the multiple selection.

It occurred to me that when setAllowsMultipleSelection is called with
false, you should unselect the selected items or, at least, add a
unselectAll method.

For now, here is my quick and dirty way to address that :

  • (void) setAllowsMultipleSelection:(BOOL)allowsMultipleSelection
    {
    _flags.allowsMultipleSelection = (allowsMultipleSelection ? 1 : 0);
    // We should unselect the items...
    if (!allowsMultipleSelection)
    {
    if ([self.selectionIndexes count]>0)
    {
    [self beginUpdates];
    int currentIndex=[self.selectionIndexes firstIndex];
    while (currentIndex!=NSNotFound)
    {
    [self deselectItemAtIndex:currentIndex animated:YES];
    currentIndex=[self.selectionIndexes
    indexGreaterThanIndex:currentIndex];
    }
    [self endUpdates];
    }
    }
    }

Frederic


Reply to this email directly or view it on
GitHubhttps://github.com//pull/126#issuecomment-7901252.

@synergie7
Copy link

I needed multiple selections, so I pulled the Dev branch.

However there appears to be a bug in that the _selectedIndices array is not cleared during reloadData.

In my case I'm deleting items from the list based on multiple selections, but I've found that if the user deletes a single item, (I then reload the gridView), then goes back and deletes another single item, the _selectedIndices.count shows 2 on the second delete attempt because selectedIndices still contains the index from the previous selection (prior to the reload).

My hack / fix for this has been to add [_selectedIndices removeAllIndexes]; to -(void) reloadData in AQGrivdView.m but I'm not sure if this is the best way to do it, or even if this functionality would suit everyone.
i.e I'm not sure if some people want the selection list to be retained even after a reload.

@chadpod
Copy link

chadpod commented Apr 22, 2013

I added support for multiple selection a while back, but am not using it anymore. Figured I would post here in case it is of any value to you guys, as I'm thinking of deleting the fork. 2964b15

@synergie7
Copy link

Hi Chadpod,

I'm using it in my app "Personal Wiki" (iPad). As I allow users to do multiple selections on items in the grid, which they can then delete.

Rather than deleting the fork and hence removing this functionality, is there any way it can be rolled into the master branch ?

@evadne
Copy link
Collaborator

evadne commented Apr 22, 2013

It seems that allowsMultipleSelection is already on master for a long while.

On Apr 22, 2013, at 2:46 PM, Roger Clark notifications@github.com wrote:

Hi Chadpod,

I'm using it in my app "Personal Wiki" (iPad). As I allow users to do multiple selections on items in the grid, which they can then delete.

Rather than deleting the fork and hence removing this functionality, is there any way it can be rolled into the master branch ?


Reply to this email directly or view it on GitHub:
#126 (comment)

@chadpod
Copy link

chadpod commented Apr 22, 2013

@evadne I see it in development, but not master? @synergie7 I'll leave my fork as is for the time being, but I recommend you move to the official branch, as I haven't maintained my implementation. Glad you found it of use anyway!

@evadne
Copy link
Collaborator

evadne commented Apr 22, 2013

Yes — it is on development indeed.

On Apr 22, 2013, at 2:58 PM, Chad Podoski notifications@github.com wrote:

@evadne I see it in development, but not master? @synergie7 I'll leave my fork as is for the time being, but I recommend you move to the official branch, as I haven't maintained my implementation. Glad you found it of use anyway!


Reply to this email directly or view it on GitHub:
#126 (comment)

@synergie7
Copy link

OK.

I'll switch to the master branch.

Thanks

@synergie7
Copy link

Sorry. I mean I'll use the Dev branch.

To be honest, its been ages since I last worked on the app, I can't really remember which branch I ended up using.
I think it was probably just the Dev branch.

I'm not sure if I used @chadpod's fork (probably not).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants