Permalink
Browse files

Fixes to previous CPButton image positioning commit.

- Checkbox and radio selected states would not show in the images. It turns out that the image and alternate image cannot be encoded->decoded->set without messing up the theme, and unfortunately the theme blending task (during a jake build) would do so. So for now this means that we cannot encode non-theme images with a button class, which is unlikely to happen.

- Since we cannot access themes in the NS classes, I went ahead and started to make some constants in a few control classes so that the values can be changed in one place and affect the theme and nib2cib.
  • Loading branch information...
1 parent 04b331a commit ce2c6628723517a573c54c26116a7bfe3203a06e Aparajita Fishman committed with Klaas Pieter Annema Sep 10, 2010
View
@@ -74,6 +74,9 @@ CPChangeBackgroundCellMask = CPBackgroundButtonMask;
CPButtonStateMixed = CPThemeState("mixed");
+CPButtonDefaultHeight = 24.0;
+CPButtonImageOffset = 3.0;
+
/*!
@ingroup appkit
@class CPButton
@@ -674,9 +677,6 @@ var CPButtonImageKey = @"CPButtonImageKey",
{
_controlSize = CPRegularControlSize;
- [self setImage:[aCoder decodeObjectForKey:CPButtonImageKey]];
- [self setAlternateImage:[aCoder decodeObjectForKey:CPButtonAlternateImageKey]];
-
_title = [aCoder decodeObjectForKey:CPButtonTitleKey];
_alternateTitle = [aCoder decodeObjectForKey:CPButtonAlternateTitleKey];
@@ -708,9 +708,6 @@ var CPButtonImageKey = @"CPButtonImageKey",
{
[super encodeWithCoder:aCoder];
- [aCoder encodeObject:[self image] forKey:CPButtonImageKey];
- [aCoder encodeObject:[self alternateImage] forKey:CPButtonAlternateImageKey];
-
[aCoder encodeObject:_title forKey:CPButtonTitleKey];
[aCoder encodeObject:_alternateTitle forKey:CPButtonAlternateTitleKey];
View
@@ -22,6 +22,7 @@
@import "CPButton.j"
+CPCheckBoxImageOffset = 4.0;
@implementation CPCheckBox : CPButton
{
View
@@ -62,6 +62,9 @@
option.
*/
+
+CPRadioImageOffset = 4.0;
+
@implementation CPRadio : CPButton
{
CPRadioGroup _radioGroup;
@@ -330,7 +330,7 @@ var themedButtonValues = nil,
+ (CPButton)makeButton
{
- return [[CPButton alloc] initWithFrame:CGRectMake(0.0, 0.0, 60.0, 24.0)];
+ return [[CPButton alloc] initWithFrame:CGRectMake(0.0, 0.0, 60.0, CPButtonDefaultHeight)];
}
+ (CPButton)button
@@ -410,10 +410,10 @@ var themedButtonValues = nil,
[@"bezel-color", defaultBezelColor, CPThemeStateBordered | CPThemeStateDefault],
[@"bezel-color", defaultHighlightedBezelColor, CPThemeStateBordered | CPThemeStateHighlighted | CPThemeStateDefault],
- [@"min-size", CGSizeMake(0.0, 24.0)],
- [@"max-size", CGSizeMake(-1.0, 24.0)],
+ [@"min-size", CGSizeMake(0.0, CPButtonDefaultHeight)],
+ [@"max-size", CGSizeMake(-1.0, CPButtonDefaultHeight)],
- [@"imageOffset", 3.0]
+ [@"imageOffset", CPButtonImageOffset]
];
[self registerThemeValues:themedButtonValues forView:button];
@@ -800,7 +800,7 @@ var themedButtonValues = nil,
[@"image", imageHighlighted, CPThemeStateHighlighted],
[@"image", imageDisabled, CPThemeStateDisabled],
[@"image", imageSelectedDisabled, CPThemeStateSelected | CPThemeStateDisabled],
- [@"imageOffset", 4.0],
+ [@"imageOffset", CPRadioImageOffset],
[@"text-color", [CPColor colorWithCalibratedWhite:79.0 / 255.0 alpha:1.0], CPThemeStateDisabled],
@@ -836,7 +836,7 @@ var themedButtonValues = nil,
[@"image", imageHighlighted, CPThemeStateHighlighted],
[@"image", imageDisabled, CPThemeStateDisabled],
[@"image", imageSelectedDisabled, CPThemeStateSelected | CPThemeStateDisabled],
- [@"imageOffset", 4.0],
+ [@"imageOffset", CPCheckBoxImageOffset],
[@"text-color", [CPColor colorWithCalibratedWhite:79.0 / 255.0 alpha:1.0], CPThemeStateDisabled],
@@ -865,7 +865,7 @@ var themedButtonValues = nil,
[@"image", mixedImage, CPButtonStateMixed],
[@"image", mixedHighlightedImage, CPButtonStateMixed | CPThemeStateHighlighted],
[@"image", mixedDisabledImage, CPButtonStateMixed | CPThemeStateDisabled],
- [@"imageOffset", 4.0, CPButtonStateMixed],
+ [@"imageOffset", CPCheckBoxImageOffset, CPButtonStateMixed],
[@"max-size", CGSizeMake(-1.0, -1.0)]
];
Oops, something went wrong.
@@ -11,7 +11,7 @@
<string key="NS.object.0">788</string>
</object>
<array class="NSMutableArray" key="IBDocument.EditedObjectIDs">
- <integer value="1182"/>
+ <integer value="372"/>
</array>
<array key="IBDocument.PluginDependencies">
<string>com.apple.InterfaceBuilder.CocoaPlugin</string>
@@ -43,7 +43,7 @@
<object class="NSTextField" id="1048195509">
<reference key="NSNextResponder" ref="439893737"/>
<int key="NSvFlags">268</int>
- <string key="NSFrame">{{40, 20}, {101, 17}}</string>
+ <string key="NSFrame">{{40, 24}, {101, 17}}</string>
<reference key="NSSuperview" ref="439893737"/>
<bool key="NSEnabled">YES</bool>
<object class="NSTextFieldCell" key="NSCell" id="889147185">
@@ -352,7 +352,7 @@ AAMAAAABAAEAAAFTAAMAAAAEAAAFwgAAAAAACAAIAAgACAABAAEAAQABA</bytes>
<object class="NSButton" id="463191555">
<reference key="NSNextResponder" ref="65589413"/>
<int key="NSvFlags">268</int>
- <string key="NSFrame">{{143, 126}, {176, 32}}</string>
+ <string key="NSFrame">{{143, 133}, {176, 25}}</string>
<reference key="NSSuperview" ref="65589413"/>
<bool key="NSEnabled">YES</bool>
<object class="NSButtonCell" key="NSCell" id="44707779">
@@ -361,8 +361,8 @@ AAMAAAABAAEAAAFTAAMAAAAEAAAFwgAAAAAACAAIAAgACAABAAEAAQABA</bytes>
<string key="NSContents">Button With Image</string>
<reference key="NSSupport" ref="688777490"/>
<reference key="NSControlView" ref="463191555"/>
- <int key="NSButtonFlags">-2038284033</int>
- <int key="NSButtonFlags2">129</int>
+ <int key="NSButtonFlags">-2034482945</int>
+ <int key="NSButtonFlags2">163</int>
<string key="NSAlternateContents"/>
<string key="NSKeyEquivalent"/>
<int key="NSPeriodicDelay">200</int>
@@ -372,7 +372,7 @@ AAMAAAABAAEAAAFTAAMAAAAEAAAFwgAAAAAACAAIAAgACAABAAEAAQABA</bytes>
<object class="NSButton" id="938838346">
<reference key="NSNextResponder" ref="65589413"/>
<int key="NSvFlags">268</int>
- <string key="NSFrame">{{24, 126}, {96, 32}}</string>
+ <string key="NSFrame">{{24, 133}, {96, 25}}</string>
<reference key="NSSuperview" ref="65589413"/>
<bool key="NSEnabled">YES</bool>
<object class="NSButtonCell" key="NSCell" id="988784972">
@@ -382,7 +382,7 @@ AAMAAAABAAEAAAFTAAMAAAAEAAAFwgAAAAAACAAIAAgACAABAAEAAQABA</bytes>
<reference key="NSSupport" ref="688777490"/>
<reference key="NSControlView" ref="938838346"/>
<int key="NSButtonFlags">-2038284033</int>
- <int key="NSButtonFlags2">129</int>
+ <int key="NSButtonFlags2">163</int>
<string key="NSAlternateContents"/>
<string key="NSKeyEquivalent"/>
<int key="NSPeriodicDelay">200</int>
@@ -111,10 +111,10 @@ var NSButtonIsBorderedMask = 0x00800000,
if ([cell isBordered])
{
- CPLog.info("Adjusting CPButton height from " +_frame.size.height+ " / " + _bounds.size.height+" to " + 24);
- _frame.size.height = 24.0;
+ CPLog.info("Adjusting CPButton height from " +_frame.size.height+ " / " + _bounds.size.height+" to " + CPButtonDefaultHeight);
+ _frame.size.height = CPButtonDefaultHeight;
_frame.origin.y += 4.0;
- _bounds.size.height = 24.0;
+ _bounds.size.height = CPButtonDefaultHeight;
}
}
else

4 comments on commit ce2c662

@cacaodev
Contributor

Ok but I still not understand why setAlternateImage: is for the highlighted state in capp (not just in CPButton, all over the fmw). What they call the "alternate state" in cocoa's NSButton doc is more like our selected state in my opinion.
Should I open an issue or there's a good reason for this i missed ?

@aparajita
Contributor

You are right, the alternate image does represent the alternate state for NSButton. But I can think of a few reasons not to change the current capp behavior:

  • In the case of buttons, there usually is no alternate state other than highlighted, and in Cocoa the highlighted state is handled automatically, whereas in capp we have to supply the highlighted state image ourselves.
  • If you change the functionality of setAlternateImage, you will break a lot of existing code (CPButton, CPMenuItem and CPToolbarItem use it), and we would have to add a separate non-Cocoa setHighlightedImage method.
  • With checkboxes and radio buttons, if you set an image/alternate image in IB, the bezel type is changed to a type that is not supported currently (NSShadowlessSquareBezel).
  • Cocoa does not have theming, capp does. The mapping of states to images is more explicit in capp than in Cocoa.

So overall, in this case I don't think Cocoa compliance would actually help, and would not be worth the disruption to the current framework.

@nickjs
Contributor
nickjs commented on ce2c662 Sep 13, 2010

Can you explain what you mean about the image encoding messing up the theme? This is really messing up my cibs, I think button images are pretty important.

@aparajita
Contributor

I found that if a custom image was defined in the nib, the normal themed images would no longer appear for any controls in the same class that did not use a custom image. There is some interaction going on between encoding/decoding and the theme that is causing this. I saw a similar thing here: http://github.com/280north/cappuccino/issues/807.

Definitely a solution needs to be found, but I'm really overloaded with other stuff right now, can you look into it?

Please sign in to comment.