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

CPRuleEditor & CPPredicateEditor fixes and enhancements #1283

Merged
merged 19 commits into from
Apr 26, 2012

Conversation

cacaodev
Copy link
Contributor

CPRuleEditor's delegate in CPRuleEditorCibTest example: fixes a bug where the predicate was not correctly handling custom control values.
CPPredicateEditor enhancements: templates merging, two-ways binding example between a text field and a predicate editor.

@cacaodev
Copy link
Contributor Author

Warning: i rebased this branch. If it's a problem i can create a new branch/pull request.

@Me1000
Copy link

Me1000 commented Jul 17, 2011

@klasspieter would you mind reviewing this? I think you know the rule editor better than anyone else on the core team.

@Me1000
Copy link

Me1000 commented Jul 24, 2011

It would probably be helpful if I got his name right :) /cc @klaaspieter

…uleEditor:displayValueForCriterion:inRow: is more restricive: you must return the same view object for a specific criterion in a given row.

In CPRuleEditorCibTest, the delegate has been updated to follow this rule, it also fixes a bug where the predicate was not correctly handling custom control values.
… item you can have different views on the right:

                      * date is (today, yesterday)
                      * date is less than [text field]

                    - CPPredicateEditorCibTest example: templates merging in the 'mixed' criterion.
                    - ojtest AppKit/CPPredicateEditorTest testing the merging feature.

                    - Cleaned and rearranged code related to the right control action method.
                    - re-enable tooltips.
…heck that was causing an exception when keyPath was a compound key path.

              CPPredicateEditor test app: bind textfield and editor to a CPObjectController.
…fter a UI interaction to prevent sending unwanted notifications to the binder.

                   Send action through the responder chain even if target or action is nil. CPControl relies on this to send updates to the binder.
…iew was not updating its frameSize when rows where added/removed.

Reported here: https://groups.google.com/d/msg/objectivej/G2MFGGSHaOY/ojcQaoNSpcAJ
I also adjusted the plus button right padding to give enough space for the scroller.
Edited CPRuleEditorCibTest xib to unhide the vertical scroller.
@Me1000
Copy link

Me1000 commented Dec 29, 2011

@aljungberg @aparajita @primalmotion @klaaspieter Any objections to merging this?

@aparajita aparajita closed this Dec 29, 2011
@aparajita aparajita reopened this Dec 29, 2011
@aparajita
Copy link
Contributor

Go for it.

@primalmotion
Copy link
Member

Seems good.

Antoine Mercadal

Le 29 déc. 2011 à 09:09, aparajita reply@reply.github.com a écrit :

Go for it.


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

@@ -1118,7 +1121,8 @@ TODO: implement
var view = [[_CPRuleEditorViewSliceDropSeparator alloc] initWithFrame:CGRectMake(0,-10, [self frame].size.width, 2)];
[view setAutoresizingMask:CPViewWidthSizable];
#if PLATFORM(DOM)
view._DOMElement.style.webkitTransition = "opacity 300ms ease-in";
if (CPBrowserIsEngine(CPWebKitBrowserEngine))
Copy link
Member

Choose a reason for hiding this comment

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

Should probably use a feature test here instead of a browser test.

@aljungberg
Copy link
Member

Looks good although I posted some minor comments above.


if (predicate != nil)
{
if ((_nestingMode == CPRuleEditorNestingModeSimple || _nestingMode == CPRuleEditorNestingModeCompound)
&& [predicate isKindOfClass:[CPComparisonPredicate class]])
predicate = [[CPCompoundPredicate alloc] initWithType:[self _compoundPredicateTypeForRootRows] subpredicates:[CPArray arrayWithObject:predicate]];

//TODO: handle CPRuleEditorNestingModeList and CPCompoundPredicate
Copy link
Contributor

Choose a reason for hiding this comment

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

Please make this an issue once the code is merged.

@klaaspieter
Copy link
Contributor

Change seems solid. I've added some minor inline notes mostly about cleaning up the code.

SmartFoldersDemo: disable the size-to-fit like feature in the controller because 1/ CPPredicateEditor should have its own sizeToFit and 2/ It was shadowing other resizing bugs.
… controller and added initWithCoder methods.
@cacaodev
Copy link
Contributor Author

cacaodev commented Apr 9, 2012

So what ?

@aparajita
Copy link
Contributor

I'm not sure what you are referring to.

@cacaodev
Copy link
Contributor Author

cacaodev commented Apr 9, 2012

I meant: So, what is the status of this pull request ?

@aljungberg
Copy link
Member

Still looks good although it no longer merges cleanly.

@aparajita
Copy link
Contributor

I'll manually merge it soon.

@aparajita
Copy link
Contributor

Looks like you are working on the merge, should I leave it to you?

Can I ask you to please run capp_lint -d strict on all your source files before submitting your final version of any code?

@cacaodev
Copy link
Contributor Author

Done. capp linted everything and resolved conflicts (at this moment 26.04.12 20:46:00).

aparajita added a commit that referenced this pull request Apr 26, 2012
CPRuleEditor & CPPredicateEditor fixes and enhancements
@aparajita aparajita merged commit 1dcd380 into cappuccino:master Apr 26, 2012
@aparajita
Copy link
Contributor

Merged, merci bien!

@cacaodev cacaodev deleted the CPRulePredicateEditor branch March 4, 2015 08:26
@cappbot cappbot added this to the Someday milestone Sep 29, 2015
@cappbot cappbot added the #new label Sep 29, 2015
@cappbot
Copy link

cappbot commented Sep 29, 2015

Milestone: Someday. Label: #new. What's next? A reviewer should examine this issue.

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

Successfully merging this pull request may close these issues.

7 participants