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

[TIMOB-24963] iOS/Android: Added new ScrollView.scrollToTop() method #9601

Merged
merged 14 commits into from Nov 15, 2017

Conversation

jquick-axway
Copy link
Contributor

Note:
This is a modified version of @m1ga's PR #9221. (Thanks Michael!)

JIRA:
Main Ticket
https://jira.appcelerator.org/browse/TIMOB-24963

Also Fixes
https://jira.appcelerator.org/browse/TIMOB-25514
https://jira.appcelerator.org/browse/TIMOB-25515

Summary:

  • Added new ScrollView.scrollToTop() method.
  • [TIMOB-25514] Fixes Android bug where vertical ScrollView.scrollTo() goes to wrong position when animated. (Introduced in Titanium 6.2.2.)
  • [TIMOB-25515] Fixes Android bug where vertical ScrollView.scrollTo() ignores "ti.ui.defaultunit" when animated.
  • Animated vertical scrolling bugs mentioned above were solved by disabling the smooth scrolling animation. It now immediately jumps to the given position. This appears to be a Google bug in their Java NestedScrollView class.

Test:

Run the below test on both Android and iOS...

  1. Build and run the below code on an Android or iOS device.
  2. An alert dialog will be displayed. Tap the "Vertical" scroll view button.
  3. Tap the scroll-to "End" button.
  4. Verify that the scroll view scrolls to the bottom.
  5. Tap the scroll-to "Middle" button.
  6. Verify that the top of the middle view is now displayed at the top of the scroll view.
  7. Tap the scroll-to "Beginning" button.
  8. Verify that the scroll view scrolls to the top.
  9. Restart the app.
  10. An alert dialog will be displayed. Tap the "Horizontal" scroll view button.
  11. Tap the scroll-to "End" button.
  12. Verify that the scroll view scrolls all the way to the right side.
  13. Tap the scroll-to "Middle" button.
  14. Verify that the left side of the middle view is now displayed at the left edge of the scroll view.
  15. Tap the scroll-to "Beginning" button.
  16. Verify that the scroll view scrolls all the way to the left side.
function showScrollView(isVertical) {
	var window = Ti.UI.createWindow();
	var scrollView = Ti.UI.createScrollView(
	{
		layout: isVertical ? "vertical" : "horizontal",
		scrollType: isVertical ? "vertical" : "horizontal",
		showHorizontalScrollIndicator: !isVertical,
		shorVerticalScrollIndicator: isVertical,
		width: Ti.UI.FILL,
		height: Ti.UI.FILL,
	});
	var firstView = Ti.UI.createLabel(
	{
		text: "First View",
		textAlign: Ti.UI.TEXT_ALIGNMENT_CENTER,
		color: "white",
		backgroundColor: "blue",
		borderColor: "white",
		borderWidth: "1dp",
		width: isVertical ? "100%" : "75%",
		height: isVertical ? "75%" : "100%",
	});
	scrollView.add(firstView);
	var middleView = Ti.UI.createLabel(
	{
		text: "Middle View",
		textAlign: Ti.UI.TEXT_ALIGNMENT_CENTER,
		color: "white",
		backgroundColor: "#008800",
		borderColor: "white",
		borderWidth: "1dp",
		width: isVertical ? "100%" : "75%",
		height: isVertical ? "75%" : "100%",
	});
	scrollView.add(middleView);
	var lastView = Ti.UI.createLabel(
	{
		text: "Last View",
		textAlign: Ti.UI.TEXT_ALIGNMENT_CENTER,
		color: "white",
		backgroundColor: "purple",
		borderColor: "white",
		borderWidth: "1dp",
		width: isVertical ? "100%" : "75%",
		height: isVertical ? "75%" : "100%",
	});
	scrollView.add(lastView);
	window.add(scrollView);
	var floatingView = Ti.UI.createView(
	{
		layout: "vertical",
		backgroundColor: "white",
		borderColor: "black",
		borderWidth: "1dp",
		bottom: "10dp",
		width: Ti.UI.SIZE,
		height: Ti.UI.SIZE,
	});
	floatingView.add(Ti.UI.createLabel(
	{
		text: "Scroll to:",
		color: "black",
		top: "5dp",
		bottom: "5dp",
		left: "10dp",
		width: Ti.UI.SIZE,
		height: Ti.UI.SIZE,
	}));
	var buttonContainer = Ti.UI.createView(
	{
		layout: "horizontal",
		bottom: "5dp",
		width: Ti.UI.SIZE,
		height: Ti.UI.SIZE,
	});
	var scrollUpButton = Ti.UI.createButton(
	{
		title: "Beginning",
		left: "5dp",
		right: "5dp",
		width: Ti.UI.SIZE,
		height: Ti.UI.SIZE,
	});
	scrollUpButton.addEventListener("click", function(e) {
		scrollView.scrollToTop();
	});
	buttonContainer.add(scrollUpButton);
	var scrollMiddleButton = Ti.UI.createButton(
	{
		title: "Middle",
		left: "5dp",
		right: "5dp",
		width: Ti.UI.SIZE,
		height: Ti.UI.SIZE,
	});
	scrollMiddleButton.addEventListener("click", function(e) {
		var x = isVertical ? 0 : firstView.size.width;
		var y = isVertical ? firstView.size.height : 0;
		scrollView.scrollTo(x, y, { animated: true });
	});
	buttonContainer.add(scrollMiddleButton);
	var scrollDownButton = Ti.UI.createButton(
	{
		title: "End",
		left: "5dp",
		right: "5dp",
		width: Ti.UI.SIZE,
		height: Ti.UI.SIZE,
	});
	scrollDownButton.addEventListener("click", function(e) {
		scrollView.scrollToBottom();
	});
	buttonContainer.add(scrollDownButton);
	floatingView.add(buttonContainer);
	window.add(floatingView);
	window.add(buttonContainer);
	window.open();
}

var dialog = Ti.UI.createAlertDialog(
{
	message: "Select ScrollView Type",
	buttonNames: ["Vertical", "Horizontal"],
});
dialog.addEventListener("click", function(e) {
	if (e.index == 0) {
		showScrollView(true);
	} else if (e.index == 1) {
		showScrollView(false);
	}
});
if ((Ti.Platform.osname === "iphone") || (Ti.Platform.osname === "ipad")) {
	var rootWindow = Ti.UI.createWindow();
	rootWindow.addEventListener("open", function(e) {
		dialog.show();
	});
	rootWindow.open();
} else {
	dialog.show();
}

m1ga and others added 10 commits July 14, 2017 20:18
[TIMOB-24963] Add iOS-parity, update docs
- [TIMOB-25514] Fixed bug where vertical ScrollView.scrollTo() goes to wrong position when animated. (Introduced in 6.2.2.)
- [TIMOB-25515] Fixed bug where ScrollView.scrollTo() ignores "ti.ui.defaultunit" when animated.
- Fixed ScrollView.scrollToTop() method for vertical scroll views for TIMOB-24963.
  * This bug was never introduced into Titanium. It was caught before the pull request was merged.
  * Caused by bug in Google's NestedScrollView which Titanium switched to in 6.2.2.
- Restored code formatting back to Axway Appcelerator coding standards.
Copy link
Collaborator

@hansemannn hansemannn left a comment

Choose a reason for hiding this comment

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

iOS approved. Waiting for Android-approval before QE can merge.

@mukherjee2 mukherjee2 removed their assignment Nov 15, 2017
@mukherjee2 mukherjee2 self-requested a review November 15, 2017 10:38
Copy link
Contributor

@mukherjee2 mukherjee2 left a comment

Choose a reason for hiding this comment

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

Tested with this environment:
Node Version: 8.9.1
NPM Version: 5.5.1
Mac OS: 10.13
Appc CLI: 7.0.0-master.13
Appc CLI NPM: 4.2.11-2
Titanium SDK version: 7.0.0.v20171114213729
Appcelerator Studio vers 4.10.0
Xcode 9.1/iOS 11.1 iPhone 7 Plus
Android 7.1.2 (Pixel), 4.1.2 (Galaxy S2)

Passed FR. I followed the steps per Josh's test case with iOS 11, and Android 7.1.2, and 4.1.2. Tests passed.

@mukherjee2
Copy link
Contributor

@garymathews can you please CR? If there are code changes, I'll redo the FR. Otherwise, I'll merge it.

Copy link
Contributor

@garymathews garymathews left a comment

Choose a reason for hiding this comment

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

CR: PASS

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

Successfully merging this pull request may close these issues.

None yet

7 participants