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

Bug fixes #1475

Merged
merged 9 commits into from Apr 25, 2013
Merged

Bug fixes #1475

merged 9 commits into from Apr 25, 2013

Conversation

pjrobertson
Copy link
Member

@pjrobertson pjrobertson commented Apr 18, 2013

Fix 3 small bugs.Maybe more to come

@skurfer
Copy link
Member

@skurfer skurfer commented Apr 18, 2013

What about selecting a new trigger when it gets created (via the preferences)? Any chance you're going to look at #1424 here? "Out of scope"? 😄

@pjrobertson
Copy link
Member Author

@pjrobertson pjrobertson commented Apr 19, 2013

I thought I fixed the selecting thing no? Hmm....

I'll take a look at #1424 in a few days (AFK again!)

On 19 April 2013 02:37, Rob McBroom notifications@github.com wrote:

What about selecting a new trigger when it gets created (via the
preferences)? Any chance you're going to look at #1424https://github.com/quicksilver/Quicksilver/issues/1424here? "Out of scope"? [image:
😄]


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

@skurfer
Copy link
Member

@skurfer skurfer commented Apr 19, 2013

You did fix the selection when the list is re-ordered. That works really well. It also seems easier to locate a new trigger, but not as easy as if it were selected by default.

@pjrobertson
Copy link
Member Author

@pjrobertson pjrobertson commented Apr 22, 2013

I hope you're happy now, 2 hours of fiddling with bindings, NSArrayControllers, QSTreeControllers (whatever they are) and IB :)

@skurfer
Copy link
Member

@skurfer skurfer commented Apr 22, 2013

2 hours? You got off easy by IB standards. 😉

I can't see any real difference. The new trigger is selected initially (when it's blank and the interface pops up asking what the trigger should do), but after you define a command and hit to set it, the selection is lost.

So I think the line you changed is doing its job, but another method is being called when the trigger is populated with its command, and that's where the selection needs to happen. I'll see if I can figure it out.

@skurfer
Copy link
Member

@skurfer skurfer commented Apr 22, 2013

Got it. Just take the line you changed:

[triggerTable selectRowIndexes:[NSIndexSet indexSetWithIndex:[[triggerArrayController arrangedObjects] indexOfObject:trigger]] byExtendingSelection:NO];

and move it from addTrigger: to the bottom of addSheetDidEnd:returnCode:contextInfo:.

@pjrobertson
Copy link
Member Author

@pjrobertson pjrobertson commented Apr 23, 2013

OK I see your point about the selecting the wrong thing after hitting . I'd only looked at selecting the right place when you first hit the '+' symbol.

I've left the line in addTrigger and added it to where you said. I like how it selects the trigger when you first hit '+' as well (try populating QS's 1st/2nd panes before adding a trigger, the current 'command' in QS get automatically added to the new trigger, so its position in the list changes depending on what's selected)

skurfer added a commit that referenced this issue Apr 25, 2013
@skurfer skurfer merged commit d1af527 into quicksilver:master Apr 25, 2013
@pjrobertson pjrobertson deleted the bugFixes branch Jul 19, 2013
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

2 participants