From 8e497e2a48af692643d91b3883654d0d1350b9d5 Mon Sep 17 00:00:00 2001 From: Martin Carlberg Date: Mon, 2 Mar 2015 16:30:02 +0100 Subject: [PATCH 1/2] =?UTF-8?q?Fixed:=20Method=20scrollRectToVisible:(CGRe?= =?UTF-8?q?ct)aRect=20in=20CPView=20didn=E2=80=99t=20work=20correctly=20if?= =?UTF-8?q?=20aRect=20is=20larger=20then=20the=20visible=20rect.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If aRect is larger then the visible rect this method scrolled all they way to the opposite edge instead of the nearest. --- AppKit/CPView.j | 47 ++++++++++++++++++++----- Tests/AppKit/CPScrollViewTest.j | 62 +++++++++++++++++++++++++++++++-- 2 files changed, 97 insertions(+), 12 deletions(-) diff --git a/AppKit/CPView.j b/AppKit/CPView.j index 12f207bd6..4b526d274 100644 --- a/AppKit/CPView.j +++ b/AppKit/CPView.j @@ -2723,18 +2723,47 @@ setBoundsOrigin: if (CGRectContainsRect(documentViewVisibleRect, rectInDocumentView)) return NO; - var scrollPoint = CGPointMakeCopy(documentViewVisibleRect.origin); + var currentScrollPoint = documentViewVisibleRect.origin, + scrollPoint = CGPointMakeCopy(currentScrollPoint), + rectInDocumentViewMinX = CGRectGetMinX(rectInDocumentView), + documentViewVisibleRectMinX = CGRectGetMinX(documentViewVisibleRect), + doesItFitForWidth = documentViewVisibleRect.size.width >= rectInDocumentView.size.width; // One of the following has to be true since our current visible rect didn't contain aRect. - if (CGRectGetMinX(rectInDocumentView) < CGRectGetMinX(documentViewVisibleRect)) - scrollPoint.x = CGRectGetMinX(rectInDocumentView); - else if (CGRectGetMaxX(rectInDocumentView) > CGRectGetMaxX(documentViewVisibleRect)) - scrollPoint.x += CGRectGetMaxX(rectInDocumentView) - CGRectGetMaxX(documentViewVisibleRect); + if (rectInDocumentViewMinX < rectInDocumentViewMinX && doesItFitForWidth) + // Scroll to left edge of aRect as it is to the left of the visible rect and it fit inside + scrollPoint.x = rectInDocumentViewMinX; + else if (CGRectGetMaxX(rectInDocumentView) > CGRectGetMaxX(documentViewVisibleRect) && doesItFitForWidth) + // Scroll to right edge of aRect as it is to the right of the visible rect and it fit inside + scrollPoint.x = CGRectGetMaxX(rectInDocumentView) - documentViewVisibleRect.size.width; + else if (rectInDocumentViewMinX > documentViewVisibleRectMinX) + // Scroll to left edge of aRect as it is to the right of the visible rect and it doesn't fit inside + scrollPoint.x = rectInDocumentViewMinX; + else if (CGRectGetMaxX(rectInDocumentView) < CGRectGetMaxX(documentViewVisibleRect)) + // Scroll to right edge of aRect as it is to the left of the visible rect and it doesn't fit inside + scrollPoint.x = CGRectGetMaxX(rectInDocumentView) - documentViewVisibleRect.size.width; - if (CGRectGetMinY(rectInDocumentView) < CGRectGetMinY(documentViewVisibleRect)) - scrollPoint.y = CGRectGetMinY(rectInDocumentView); - else if (CGRectGetMaxY(rectInDocumentView) > CGRectGetMaxY(documentViewVisibleRect)) - scrollPoint.y += CGRectGetMaxY(rectInDocumentView) - CGRectGetMaxY(documentViewVisibleRect); + var rectInDocumentViewMinY = CGRectGetMinY(rectInDocumentView), + documentViewVisibleRectMinY = CGRectGetMinY(documentViewVisibleRect), + doesItFitForHeight = documentViewVisibleRect.size.height >= rectInDocumentView.size.height; + + if (rectInDocumentViewMinY < documentViewVisibleRectMinY && doesItFitForHeight) + // Scroll to top edge of aRect as it is above the visible rect and it fit inside + scrollPoint.y = rectInDocumentViewMinY; + else if (CGRectGetMaxY(rectInDocumentView) > CGRectGetMaxY(documentViewVisibleRect) && doesItFitForHeight) + // Scroll to bottom edge of aRect as it is below the visible rect and it fit inside + scrollPoint.y = CGRectGetMaxY(rectInDocumentView) - documentViewVisibleRect.size.height; + else if (rectInDocumentViewMinY > documentViewVisibleRectMinY) + // Scroll to top edge of aRect as it is below the visible rect and it doesn't fit inside + scrollPoint.y = rectInDocumentViewMinY; + else if (CGRectGetMaxY(rectInDocumentView) < CGRectGetMaxY(documentViewVisibleRect)) + // Scroll to bottom edge of aRect as it is above the visible rect and it doesn't fit inside + scrollPoint.y = CGRectGetMaxY(rectInDocumentView) - documentViewVisibleRect.size.height; + + // Don't scroll if aRect contains the whole visible rect as it is already as visible as possible. + // We check this by comparing to new scrollPoint to the current. + if (CGPointEqualToPoint(scrollPoint, currentScrollPoint)) + return NO; [enclosingClipView scrollToPoint:scrollPoint]; diff --git a/Tests/AppKit/CPScrollViewTest.j b/Tests/AppKit/CPScrollViewTest.j index 379cea865..40a92749e 100644 --- a/Tests/AppKit/CPScrollViewTest.j +++ b/Tests/AppKit/CPScrollViewTest.j @@ -242,9 +242,11 @@ [self assertPoint:CGPointMake(0, 0) equals:visibleRect.origin message:@"VisibleRect origin not at top left corner"]; // Make the second text field visible - [textField2 scrollRectToVisible:[textField2 bounds]]; + var hasScrolled = [textField2 scrollRectToVisible:[textField2 bounds]]; - var visibleRectOriginShouldBeAt = CGPointMake(500 - originalVisibleSize.width + textField2Size.width, 500 -originalVisibleSize.height + textField2Size.height); + [self assertTrue:hasScrolled]; + + var visibleRectOriginShouldBeAt = CGPointMake(500 - originalVisibleSize.width + textField2Size.width, 500 - originalVisibleSize.height + textField2Size.height); visibleRect = [documentView visibleRect]; @@ -252,12 +254,66 @@ [self assertPoint:visibleRectOriginShouldBeAt equals:visibleRect.origin message:@"Second text field not at lower right corner in visible rect"]; // Make the first text field visible again - [textField1 scrollRectToVisible:[textField2 bounds]]; + hasScrolled = [textField1 scrollRectToVisible:[textField1 bounds]]; + [self assertTrue:hasScrolled]; visibleRect = [documentView visibleRect]; // We should now be back at top left corner [self assertPoint:CGPointMake(0, 0) equals:visibleRect.origin message:@"VisibleRect origin not at top left corner again"]; + + // Try to scroll again and it should not scroll + hasScrolled = [textField1 scrollRectToVisible:[textField1 bounds]]; + [self assertFalse:hasScrolled]; +} + +- (void)testScrollRectToVisibleWithLargeRect +{ + var scrollView = [[CPScrollView alloc] initWithFrame:CGRectMake(0, 0, 100, 100)], + documentView = [[CPView alloc] initWithFrame:CGRectMake(0, 0, 1000, 1000)], + view1 = [[CPView alloc] initWithFrame:CGRectMake(0, 0, 200, 200)], + view2 = [[CPView alloc] initWithFrame:CGRectMake(500, 500, 200, 200)], + view1Size = CGSizeMakeCopy([view1 bounds].size), + view2Size = CGSizeMakeCopy([view2 bounds].size); + + [scrollView setDocumentView:documentView]; + + [view1 setFrameOrigin:CGPointMake(0, 0)]; + [view2 setFrameOrigin:CGPointMake(500, 500)]; + + [documentView addSubview:view1]; + [documentView addSubview:view2]; + + var visibleRect = [documentView visibleRect], + originalVisibleSize = CGSizeMakeCopy(visibleRect.size); + + // Make sure we are at the top left corner + [self assertPoint:CGPointMake(0, 0) equals:visibleRect.origin message:@"VisibleRect origin not at top left corner"]; + + // Make the second view visible + var hasScrolled = [view2 scrollRectToVisible:[view2 bounds]]; + + [self assertTrue:hasScrolled]; + + visibleRect = [documentView visibleRect]; + + // We should now have the origin of view2 in the upper left corner + [self assertPoint:CGPointMake(500, 500) equals:visibleRect.origin message:@"Origin of second view not at upper left corner in visible rect"]; + + // Make the first view visible again + hasScrolled = [view1 scrollRectToVisible:[view1 bounds]]; + [self assertTrue:hasScrolled]; + + visibleRect = [documentView visibleRect]; + + var visibleRectOriginShouldBeAt = CGPointMake(200 - visibleRect.size.width, 200 - visibleRect.size.height); + + // We should now be back almost at top left corner except that the lower right corner of the rect should be at the lower right corner of the visible rect. + [self assertPoint:visibleRectOriginShouldBeAt equals:visibleRect.origin message:@"VisibleRect origin not at top left corner again"]; + + // Try to scroll again and it should not scroll even as some parts are outside the visible rect + hasScrolled = [view1 scrollRectToVisible:[view1 bounds]]; + [self assertFalse:hasScrolled]; } -(void)testNotificationsRegistered From b3dfce7b5aff6a4ed3bb3f39cb1009423dc1b84d Mon Sep 17 00:00:00 2001 From: Martin Carlberg Date: Tue, 31 Mar 2015 17:30:57 +0200 Subject: [PATCH 2/2] Fixed: Compared the rects minimum x value with it self instead of the visible minimum x value. Also fixed the test case to catch this. --- AppKit/CPView.j | 2 +- Tests/AppKit/CPScrollViewTest.j | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/AppKit/CPView.j b/AppKit/CPView.j index 4b526d274..324128a0c 100644 --- a/AppKit/CPView.j +++ b/AppKit/CPView.j @@ -2730,7 +2730,7 @@ setBoundsOrigin: doesItFitForWidth = documentViewVisibleRect.size.width >= rectInDocumentView.size.width; // One of the following has to be true since our current visible rect didn't contain aRect. - if (rectInDocumentViewMinX < rectInDocumentViewMinX && doesItFitForWidth) + if (rectInDocumentViewMinX < documentViewVisibleRectMinX && doesItFitForWidth) // Scroll to left edge of aRect as it is to the left of the visible rect and it fit inside scrollPoint.x = rectInDocumentViewMinX; else if (CGRectGetMaxX(rectInDocumentView) > CGRectGetMaxX(documentViewVisibleRect) && doesItFitForWidth) diff --git a/Tests/AppKit/CPScrollViewTest.j b/Tests/AppKit/CPScrollViewTest.j index 40a92749e..320ecb263 100644 --- a/Tests/AppKit/CPScrollViewTest.j +++ b/Tests/AppKit/CPScrollViewTest.j @@ -229,7 +229,7 @@ [scrollView setDocumentView:documentView]; - [textField1 setFrameOrigin:CGPointMake(0, 0)]; + [textField1 setFrameOrigin:CGPointMake(10, 10)]; [textField2 setFrameOrigin:CGPointMake(500, 500)]; [documentView addSubview:textField1]; @@ -260,7 +260,7 @@ visibleRect = [documentView visibleRect]; // We should now be back at top left corner - [self assertPoint:CGPointMake(0, 0) equals:visibleRect.origin message:@"VisibleRect origin not at top left corner again"]; + [self assertPoint:CGPointMake(10, 10) equals:visibleRect.origin message:@"VisibleRect origin not at top left corner again"]; // Try to scroll again and it should not scroll hasScrolled = [textField1 scrollRectToVisible:[textField1 bounds]];