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-15249] Android: TableView header and footer cannot be removed or resized dynamically #9193

Merged
merged 13 commits into from May 21, 2018

Conversation

drauggres
Copy link
Contributor

https://jira.appcelerator.org/browse/TIMOB-15249

[TIMOB-15249] unset headerView and footerView in tableView on android

@hansemannn hansemannn changed the title [TIMOB-15249] unset header-, footer- in tableView [TIMOB-15249] Anroid: headerView & footerView cannot be removed or resized after creation Jul 4, 2017
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

TEST CASE
var win = Ti.UI.createWindow({backgroundColor: 'gray'}),
    headerView = Ti.UI.createView({
        backgroundColor: 'red',
        height: 100,
        width: Ti.UI.FILL
    }),
    footerView = Ti.UI.createView({
        backgroundColor: 'green',
        height: 100,
        width: Ti.UI.FILL
    }),
    table = Ti.UI.createTableView({
        headerView: headerView,
        footerView: footerView,
        data: [
            {title: 'ITEM'}
        ]
    });

setTimeout(function() {
    table.headerView = null;
    table.footerView = null;
}, 3000);

win.add(table);
win.open();

@ssjsamir
Copy link
Contributor

ssjsamir commented Jul 12, 2017

@garymathews When using the following test case: (https://jira.appcelerator.org/browse/TIMOB-15249)

var w = Ti.UI.createWindow({
backgroundColor: 'gray'
});
 
var t = Ti.UI.createTableView({
data: [
{ title: 'One' },
{ title: 'Two' },
{ title: 'Three' },
{ title: 'Four' },
{ title: 'Five' }
]
});
 
var h = Ti.UI.createView({
backgroundColor: 'red',
height: 25,
width: Ti.UI.FILL
});
 
h.addEventListener('click', function () {
h.height = 50;
f.height = 50;
});
 
var f = Ti.UI.createView({
backgroundColor: 'green',
height: 25,
width: Ti.UI.FILL
});
 
f.addEventListener('click', function () {
t.headerView = null;
t.footerView = null;
});
 
t.headerView = h;
t.footerView = f;
 
w.add(t);
w.open();

I am able to close the headerView and footerView when pressing on green but I am unable to resize it when pressing on red.

Test Environement
Appcelerator Command-Line Interface, version 6.2.2
Google Nexus 6P (7.1.1)
Operating System Name: Mac OS X El Capitan
Operating System Version: 10.11.6
Node.js Version: 6.10.1
Xcode: 8.2
Appcelerator Studio: 4.9.0.201705251638

@sgtcoolguy sgtcoolguy added this to the 7.1.0 milestone Nov 21, 2017
@sgtcoolguy
Copy link
Contributor

@garymathews @drauggres So it sounds like this PR supports explicitly removing or replacing the header/footer view, but not resizing it? Looking at the changes and the test case @ssjsamir mentions, do we need to hook any sort of listeners to the header/footer view to adjust to changes to some subset of it's properties (as here we adjust the height)?

@sgtcoolguy sgtcoolguy changed the title [TIMOB-15249] Anroid: headerView & footerView cannot be removed or resized after creation [TIMOB-15249] Android: headerView & footerView cannot be removed or resized after creation Nov 21, 2017
@drauggres
Copy link
Contributor Author

@sgtcoolguy, PR adds only removing support. I didn't find a way to inform TableView about header/footer height change.

@longton95 longton95 requested review from longton95 and removed request for ssjsamir January 16, 2018 16:54
@hansemannn hansemannn modified the milestones: 7.1.0, 7.2.0 Feb 26, 2018
@lokeshchdhry
Copy link
Contributor

@garymathews , As this PR just adds support to remove the header & footer view & the ticket says resize as well. Is the PR complete & ready for an FR & closing ?

@sgtcoolguy sgtcoolguy modified the milestones: 7.2.0, 7.3.0 May 16, 2018
@build
Copy link
Contributor

build commented May 17, 2018

Messages
📖

🎉 Another contribution from our awesome community member, drauggres! Thanks again for helping us make Titanium SDK better. 👍

📖

💾 Here's the generated SDK zipfile.

Generated by 🚫 dangerJS

@hansemannn
Copy link
Collaborator

@lokeshchdhry This PR should be fine. The ticket use case meant to have resizing (width = 0) as an alternative to removing it, but having the ability to remove it is an ideal solution.

@drauggres
Copy link
Contributor Author

@hansemannn Actually in latest SDK we can't remove footer and header from tableView ("null
object reference", this fixed in first commit, but resizing wasn't there when you posted your comment.
Now resizing should be fine too.

@hansemannn
Copy link
Collaborator

hansemannn commented May 17, 2018

@drauggres You can add the following unit-test and verify if it works already (may be linting/timeout errors on the initial draft):

tests/ti.ui.tableview.addontest.js

/*
 * Appcelerator Titanium Mobile
 * Copyright (c) 2015-Present by Appcelerator, Inc. All Rights Reserved.
 * Licensed under the terms of the Apache Public License
 * Please see the LICENSE included with this distribution for details.
 */
/* eslint-env mocha */
/* global Ti */
/* eslint no-unused-expressions: "off" */
'use strict';
var should = require('./utilities/assertions');

describe('Titanium.UI.TableView', function () {
	it('Add and remove headerView/footerView ', function (finish) {
		var win = Ti.UI.createWindow({ backgroundColor: 'gray' }),
		headerView = Ti.UI.createView({
			backgroundColor: 'red',
			height: 100,
			width: Ti.UI.FILL
		}),
		footerView = Ti.UI.createView({
			backgroundColor: 'green',
			height: 100,
			width: Ti.UI.FILL
		}),
		table = Ti.UI.createTableView({
			headerView: headerView,
			footerView: footerView,
			data: [
				{ title: 'ITEM' }
			]
		});

		win.addEventListener('open', function () {
			table.headerView = null;
			table.footerView = null;

			finish();
		});

		win.add(table);
		win.open();
	});
});

NOTE: There currently is a addon-test for the table-view already, which I just migrated. Once #10057 is merged, you can commit your file.

@drauggres
Copy link
Contributor Author

Removing works. I will add your test later (tests/Resources/ti.ui.tableview.addontest.js I assume).
Not sure if it's possible to write test on resizing case.

@hansemannn hansemannn merged commit 9ac2c36 into tidev:master May 21, 2018
@jquick-axway jquick-axway changed the title [TIMOB-15249] Android: headerView & footerView cannot be removed or resized after creation [TIMOB-15249] Android: TableView header and footer cannot be added/removed dynamically May 21, 2018
@drauggres drauggres deleted the TIMOB-15249 branch May 22, 2018 15:14
@jquick-axway
Copy link
Contributor

@ssjsamir, please re-run the following header/footer resize test to verify that it was fixed. From looking at the posts, it's the only issue that we haven't confirmed fixed yet. Thanks.
#9193 (comment)

@jquick-axway jquick-axway changed the title [TIMOB-15249] Android: TableView header and footer cannot be added/removed dynamically [TIMOB-15249] Android: TableView header and footer cannot be removed or resized dynamically May 23, 2018
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

9 participants