From fd3acc456d333610a7ec509d61a7560255fdd750 Mon Sep 17 00:00:00 2001 From: cacaodev Date: Mon, 20 Aug 2018 10:36:59 +0200 Subject: [PATCH] FIXED: Before this commit, CPScrollerView was not correctly differentiating the scroller-width attribute (#2740) between the vertical scroller and the horizontal scroller. The scroller width was picked from a common CPScroller instance cached in CPScroller. The bug was not visible because the method -_adjustScrollerSize was correcting the size repeatidly in the layout pass. This was causing unnecessary calls to setFrame: in a performance sensitive method (triggered by scroll events). The ScrollView is now picking the right width from the theme and a setFrame: call is skipped. Tests: Manual/CPScrollView2/ TODO : The _adjustScrollerSize method should be enabled with a flag only when a width change is explicitely requested. --- AppKit/CPScrollView.j | 30 ++++++++++++++---------------- AppKit/CPScroller.j | 7 +++++++ AppKit/_CPPopUpList.j | 10 +++++----- 3 files changed, 26 insertions(+), 21 deletions(-) diff --git a/AppKit/CPScrollView.j b/AppKit/CPScrollView.j index c59178b2f..39a884307 100644 --- a/AppKit/CPScrollView.j +++ b/AppKit/CPScrollView.j @@ -168,14 +168,13 @@ var CPScrollerStyleGlobal = CPScrollerStyleOverlay, + (CGSize)contentSizeForFrameSize:(CGSize)frameSize hasHorizontalScroller:(BOOL)hFlag hasVerticalScroller:(BOOL)vFlag borderType:(CPBorderType)borderType { - var bounds = [self _insetBounds:CGRectMake(0.0, 0.0, frameSize.width, frameSize.height) borderType:borderType], - scrollerWidth = [CPScroller scrollerWidth]; + var bounds = [self _insetBounds:CGRectMake(0.0, 0.0, frameSize.width, frameSize.height) borderType:borderType]; if (hFlag) - bounds.size.height -= scrollerWidth; + bounds.size.height -= [_horizontalScroller scrollerWidth]; if (vFlag) - bounds.size.width -= scrollerWidth; + bounds.size.width -= [_verticalScroller scrollerWidth]; return bounds.size; } @@ -185,14 +184,13 @@ var CPScrollerStyleGlobal = CPScrollerStyleOverlay, var bounds = [self _insetBounds:CGRectMake(0.0, 0.0, contentSize.width, contentSize.height) borderType:borderType], widthInset = contentSize.width - bounds.size.width, heightInset = contentSize.height - bounds.size.height, - frameSize = CGSizeMake(contentSize.width + widthInset, contentSize.height + heightInset), - scrollerWidth = [CPScroller scrollerWidth]; + frameSize = CGSizeMake(contentSize.width + widthInset, contentSize.height + heightInset); if (hFlag) - frameSize.height += scrollerWidth; + frameSize.height += [_horizontalScroller scrollerWidth]; if (vFlag) - frameSize.width += scrollerWidth; + frameSize.width += [_verticalScroller scrollerWidth]; return frameSize; } @@ -529,8 +527,8 @@ Notifies the delegate when the scroll view has finished scrolling. { var bounds = [self _insetBounds]; - [self setHorizontalScroller:[[CPScroller alloc] initWithFrame:CGRectMake(0.0, 0.0, MAX(CGRectGetWidth(bounds), [CPScroller scrollerWidthInStyle:_scrollerStyle] + 1), [CPScroller scrollerWidthInStyle:_scrollerStyle])]]; - [[self horizontalScroller] setFrameSize:CGSizeMake(CGRectGetWidth(bounds), [CPScroller scrollerWidthInStyle:_scrollerStyle])]; + [self setHorizontalScroller:[[CPScroller alloc] initWithFrame:CGRectMake(0.0, 0.0, MAX(CGRectGetWidth(bounds), [_horizontalScroller scrollerWidth] + 1), [_horizontalScroller scrollerWidth])]]; + [[self horizontalScroller] setFrameSize:CGSizeMake(CGRectGetWidth(bounds), [_horizontalScroller scrollerWidth])]; } [self reflectScrolledClipView:_contentView]; @@ -594,8 +592,8 @@ Notifies the delegate when the scroll view has finished scrolling. { var bounds = [self _insetBounds]; - [self setVerticalScroller:[[CPScroller alloc] initWithFrame:CGRectMake(0.0, 0.0, [CPScroller scrollerWidthInStyle:_scrollerStyle], MAX(CGRectGetHeight(bounds), [CPScroller scrollerWidthInStyle:_scrollerStyle] + 1))]]; - [[self verticalScroller] setFrameSize:CGSizeMake([CPScroller scrollerWidthInStyle:_scrollerStyle], CGRectGetHeight(bounds))]; + [self setVerticalScroller:[[CPScroller alloc] initWithFrame:CGRectMake(0.0, 0.0, [_verticalScroller scrollerWidth], MAX(CGRectGetHeight(bounds), [_verticalScroller scrollerWidth] + 1))]]; + [[self verticalScroller] setFrameSize:CGSizeMake([_verticalScroller scrollerWidth], CGRectGetHeight(bounds))]; } [self reflectScrolledClipView:_contentView]; @@ -911,8 +909,8 @@ Notifies the delegate when the scroll view has finished scrolling. bottomCornerFrame.origin.x = CGRectGetMinX(verticalFrame); bottomCornerFrame.origin.y = CGRectGetMaxY(verticalFrame); - bottomCornerFrame.size.width = [CPScroller scrollerWidthInStyle:_scrollerStyle]; - bottomCornerFrame.size.height = [CPScroller scrollerWidthInStyle:_scrollerStyle]; + bottomCornerFrame.size.width = [_verticalScroller scrollerWidth]; + bottomCornerFrame.size.height = [_horizontalScroller scrollerWidth]; return bottomCornerFrame; } @@ -1124,8 +1122,8 @@ Notifies the delegate when the scroll view has finished scrolling. contentFrame.size.height -= headerClipViewHeight; var difference = CGSizeMake(CGRectGetWidth(documentFrame) - CGRectGetWidth(contentFrame), CGRectGetHeight(documentFrame) - CGRectGetHeight(contentFrame)), - verticalScrollerWidth = [CPScroller scrollerWidthInStyle:[_verticalScroller style]], - horizontalScrollerHeight = [CPScroller scrollerWidthInStyle:[_horizontalScroller style]], + verticalScrollerWidth = [_verticalScroller scrollerWidth], + horizontalScrollerHeight = [_horizontalScroller scrollerWidth], hasVerticalScroll = difference.height > 0.0, hasHorizontalScroll = difference.width > 0.0, shouldShowVerticalScroller = _hasVerticalScroller && (!_autohidesScrollers || hasVerticalScroll), diff --git a/AppKit/CPScroller.j b/AppKit/CPScroller.j index ae4ed6fbc..f06d974fb 100644 --- a/AppKit/CPScroller.j +++ b/AppKit/CPScroller.j @@ -770,6 +770,13 @@ CPThemeStateScrollerKnobDark = CPThemeState("scroller-knob-dark"); _timerFadeOut = [CPTimer scheduledTimerWithTimeInterval:1.2 target:self selector:@selector(_performFadeOut:) userInfo:nil repeats:NO]; } +- (float)scrollerWidth +{ + if (_style == CPScrollerStyleLegacy) + return [self valueForThemeAttribute:@"scroller-width" inState:CPThemeStateScrollViewLegacy]; + + return [self currentValueForThemeAttribute:@"scroller-width"]; +} #pragma mark - #pragma mark Delegates diff --git a/AppKit/_CPPopUpList.j b/AppKit/_CPPopUpList.j index 6ee4e1ff0..7dfa0a010 100644 --- a/AppKit/_CPPopUpList.j +++ b/AppKit/_CPPopUpList.j @@ -233,14 +233,14 @@ var ListColumnIdentifier = @"1"; // Start with a default size, we will resize it later var frame = CGRectMake(0, 0, 200, 200); - _tableColumn = [[CPTableColumn alloc] initWithIdentifier:ListColumnIdentifier]; - [_tableColumn setWidth:CGRectGetWidth(frame) - [CPScroller scrollerWidth]]; - [_tableColumn setResizingMask:CPTableColumnAutoresizingMask]; - [_tableView addTableColumn:_tableColumn]; - _scrollView = [self makeScrollViewWithFrame:CGRectMake(0, 0, CGRectGetWidth(frame), CGRectGetHeight(frame))]; [_scrollView setDocumentView:_tableView]; + _tableColumn = [[CPTableColumn alloc] initWithIdentifier:ListColumnIdentifier]; + [_tableColumn setWidth:CGRectGetWidth(frame) - [[_scrollView verticalScroller] scrollerWidth]]; + [_tableColumn setResizingMask:CPTableColumnAutoresizingMask]; + [_tableView addTableColumn:_tableColumn]; + // This has to be done after setDocumentView so that the table knows which scroll view to update [_tableView setHeaderView:nil];