[TIMOB-16355] iOS: separatorInsets should not be used for header/section titles

GitHub Issue	n/a
Type	Bug
Priority	Medium
Status	Closed
Resolution	Fixed
Resolution Date	2016-03-14T22:19:25.000+0000
Affected Version/s	Release 3.2.0
Fix Version/s	Release 5.2.0, Release 5.2.1
Components	iOS
Labels	listview, qe-5.2.0, separatorinsets, tableview
Reporter	Fokke Zandbergen
Assignee	Angel Petkov
Created	2014-02-02T19:16:45.000+0000
Updated	2016-03-24T22:31:18.000+0000

Description

The new separatorInsets can only be set for the full TableView or ListView. Unfortunately, they affect not only the separator lines, but also the section headerTitle and footerTitle while at the same time the TableView and ListView direct headerTitle and footerTitle are **not** affected. I would suggest changing the behavior for the section headerTitle and sectionTitle to be not affected as well. You can always use a custom header and footer view and the main reason for seperatorInsets is to disable/customize the inset of the separator line in iOS7 and up.

To reproduce

Run both of this app.js both with and without the separatorInsets property and compare the screenshots attached:

var win = Ti.UI.createWindow({
	backgroundColor: '#fff'
});

var sectionFruit = Ti.UI.createTableViewSection({ headerTitle: 'Fruit headerTitle', footerTitle: 'Fruit footerTitle' });
sectionFruit.add(Ti.UI.createTableViewRow({ title: 'Apples' }));
sectionFruit.add(Ti.UI.createTableViewRow({ title: 'Bananas' }));

var sectionVeg = Ti.UI.createTableViewSection({ headerTitle: 'Vegetables headerTitle', footerTitle: 'Vegetables footerTitle' });
sectionVeg.add(Ti.UI.createTableViewRow({ title: 'Carrots' }));
sectionVeg.add(Ti.UI.createTableViewRow({ title: 'Potatoes' }));

var table = Ti.UI.createTableView({
	top: 20,
	style: Ti.UI.iPhone.TableViewStyle.GROUPED,
	headerTitle: 'Table headerTitle',
	footerTitle: 'Table footerTitle',

        // On second run remove this property
	separatorInsets: {
		left: 0, right: 0
	},

	data: [sectionFruit, sectionVeg]
});

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

Attachments

File	Date	Size
iOS-simulatorschermafbeelding 2 feb. 2014 20.10.58.png	2014-02-02T19:16:45.000+0000	57323
iOS-simulatorschermafbeelding 2 feb. 2014 20.16.18.png	2014-02-02T19:16:45.000+0000	57529

Comments

Ritu Agrawal 2014-02-03

Moving this ticket to engineering as I can reproduce this issue with 3.2.0 release.
Shameer Jan 2014-08-22

Issue reproduced with SDK Version 3.2.0 GA, 3.2.2 GA and SDK 3.4.0 GA CLI version 3.3.0, Titanium SDK version 3.4.0 Titanium SDK version 3.2.0 iOS SDK: 7.1 Appcelerator Studio, build: 3.3.0.201407111535
Fokke Zandbergen 2015-10-18

Wouldn't it be fixed if we only apply the insets on the cells and not the table view?
Hans Knöchel 2015-10-18

That would disallow the user to set the section insets if he wants. But rather be the "quick and dirty" way.
Fokke Zandbergen 2015-10-19

I remember that this property was only implemented for users to be able to revert the change in iOS 8 where it started showing these line insets by default. This is also in the docs: {quote}In iOS 7 and later, cell separators do not extend all the way to the edge of the list view. This property sets the default inset for all cells in the table.{quote} So I don't think it's a problem if we make it not apply to the section header/footer titles anymore.
Hans Knöchel 2015-11-02

Alright, let's get this into 5.2.0. [~cng] ok?

Angel Petkov 2015-12-02

Implemented rowSeparatorInsets and tableSeparatorInsets properties. If the rowSeparatorInsets is not set upon creation the inset for the cell will be the same as the header and footer as it is currently. If it's set however, it will set the cell inset to the left and right values of rowSeparatorInsets. The property can be set upon creation only because its applied to each cell as they are created. PR pending https://github.com/appcelerator/titanium_mobile/pull/7534/files.

   var win = Ti.UI.createWindow({
     backgroundColor: '#fff'
   });
    
   var sectionFruit = Ti.UI.createTableViewSection({ headerTitle: 'Fruit headerTitle', footerTitle: 'Fruit footerTitle' });
   sectionFruit.add(Ti.UI.createTableViewRow({ title: 'Apples' }));
   sectionFruit.add(Ti.UI.createTableViewRow({ title: 'Bananas' }));
    
   var sectionVeg = Ti.UI.createTableViewSection({ headerTitle: 'Vegetables headerTitle', footerTitle: 'Vegetables footerTitle' });
   sectionVeg.add(Ti.UI.createTableViewRow({ title: 'Carrots' }));
   sectionVeg.add(Ti.UI.createTableViewRow({ title: 'Potatoes' }));
    
   var table = Ti.UI.createTableView({
     top: 20,
     style: Ti.UI.iPhone.TableViewStyle.GROUPED,
     headerTitle: 'Table headerTitle',
     footerTitle: 'Table footerTitle',
    
     //       // On second run remove this property
     separatorInsets: {
       left: 100, right: 100
     },
   
     rowSeparatorInsets: {
       left: 30, right: 30
     },
    
     data: [sectionFruit, sectionVeg]
   });
   
   table.addEventListener("scrollend",function(e) {
     Ti.API.info("Inside event");
     table.setRowSeparatorInsets({left: 50, right:50});
   });
    
   win.add(table);
   win.open();

Chee Kiat Ng 2015-12-03

[~apetkov] so according to your sample code, the expected behavior of setRowSeperatorInsets in the event "scrollend" is that it does nothing correct?
Angel Petkov 2015-12-03

Yeah it should have no effect to the behavior.
Chee Kiat Ng 2015-12-04

CR and FT passed. PR Merged!
Harry Bryant 2016-02-02

Verified as fixed, setting rowSeparatorInsets property does not influence the position of header / section titles, if the property is not set - but instead separatorInsets - header / section titles will be influenced. Tested on: iPhone 6s Plus Device (9.2) Mac OSX El Capitan 10.11 (15A284) Appc Studio: 4.5.0.201601262138 Ti SDK: 5.2.0.v20160202103508 Appc NPM: 4.2.3-1 App CLI: 5.2.0-239 Xcode 7.2 Node v4.2.3 *Closing Ticket.*
Hans Knöchel 2016-02-27

Reopening the ticket, because the deprecation warning Ti.UI.TableView.separatorInsets DEPRECATED in 6.0.0, in favor of Ti.UI.TableView.tableSeparatorInsets is currently (5.2.0.GA) shown. Fix + backport to adjust the deprecation: PR (master): https://github.com/appcelerator/titanium_mobile/pull/7782 PR (5_2_X): https://github.com/appcelerator/titanium_mobile/pull/7783 Steps to test: Run the above demo code using 5.2.0.GA, 5.2.x and master branch, check the logs.
Eric Wieber 2016-03-01

Verified fixed, using: MacOS 10.11.3 (15D21) Studio 4.5.0.201602170821 Ti SDK 5.2.1.v20160228190750 Appc NPM 4.2.3 Appc CLI 5.2.0 Alloy 1.7.33 Xcode 7.2 (7C68) Behavior is as Harry describes, however the deprecation warning now properly states that Ti.UI.TableView.separatorInsets are deprecated in 5.2.0. Tested using provided test code
Fokke Zandbergen 2016-03-10

Reopening since the issue calls for both TableView and ListView to be fixed but the merged PRs only contain fixes for TableView.

Angel Petkov 2016-03-10

PR(Master):https://github.com/appcelerator/titanium_mobile/pull/7843 PR(5_2_X):https://github.com/appcelerator/titanium_mobile/pull/7829 Demo code:

    var win = Ti.UI.createWindow({
      backgroundColor: '#fff'
    });
    var sections = [];
    var sectionFruit = Ti.UI.createListSection({ headerTitle: 'Fruit headerTitle',});
    var fruitDataSet = [
        {properties: { title: 'Apple'}},
        {properties: { title: 'Banana'}},
    ];
    sectionFruit.setItems(fruitDataSet);
    sections.push(sectionFruit);
     
    var sectionVeg = Ti.UI.createListSection({ headerTitle: 'Vegetables headerTitle'});
    var vegDataSet = [
        {properties: { title: 'Carrots'}},
        {properties: { title: 'Potatoes'}},
    ];
    sectionVeg.setItems(vegDataSet);
    sections.push(sectionVeg);
     
    var listView = Ti.UI.createListView({
      top: 20,
      headerTitle: 'Table headerTitle',
      footerTitle: 'Table footerTitle',
      
        separatorInsets: {
        left: 50, right: 50
      },
     
      rowSeparatorInsets: {
        left: 30, right: 30
      },
      
    });
     listView.sections = sections;
     listView.addEventListener("itemclick",function(e){
      Ti.API.info("here");
      listView.setSeparatorInsets({left: 50, right: 50});
    
     });
     
    win.add(listView);
    win.open();

Ingo Muschenetz 2016-03-14

Bump. Review?
Eric Wieber 2016-03-16

A couple things noticed when using the most recent sample code, with this environment: MacOS 10.11.3 (15D21) Studio 4.5.0.201602170821 Ti SDK 5.2.1.v20160316113102 Appc NPM 4.2.4-2 Appc CLI 5.2.1-12 Alloy 1.7.35 Xcode 7.2.1 (7C1002) 1. The property added to ListView is tableSeparatorInsets. I would expect the property be called listSeparatorInsets. Anyone agree? 2. I do not see the deprecation warning when using separatorInsets with ListViews, as I do with TableViews.
Angel Petkov 2016-03-17

[~ewieber] Hi, yeah i agree we should change the name to listSeparatorInsets, ill do a PR to change the name and updated the docs. As for the deprecated message it should definitely be there just tested. Its there for the master version [master](https://github.com/appcelerator/titanium_mobile/blob/master/iphone/Classes/TiUIListView.m#L549) and [5.2.X](https://github.com/appcelerator/titanium_mobile/blob/5_2_X/iphone/Classes/TiUIListView.m#L543)
Eric Wieber 2016-03-17

[~apetkov] I do not see the deprecation message when I have SDK 5.2.1.X selected from the CLI, no matter what the tiapp.xml SDK is set to. If I select 5.4.X from the CLI, then I will see the message
Hans Knöchel 2016-03-18
Eric Wieber 2016-03-24

Verified fixed, using: MacOS 10.11.4 (15E65) Studio 4.5.0.201602170821 Ti SDK 5.2.1.v20160318225121 Appc NPM 4.2.4-2 Appc CLI 5.2.1-21 Alloy 1.8.2 Xcode 7.3 (7D175) I now correctly see the deprecation warning and separatorInsets has been split into two properties (tableSeparatorInsets and rowSeparatorInsets) for tableViews and listViews. Tested using provided sample code, with edits

JSON Source