nuno / TiCollectionView

UICollectionView / GridView for Appcelerator Titanium
MIT License
111 stars 43 forks source link

Move to ARC, update libraries, fix all warnings & errors, bump 2.0.0 #66

Closed hansemannn closed 7 years ago

hansemannn commented 7 years ago

Work in Progress

Current build: ti.collectionview-iphone-2.0.0.zip

hansemannn commented 7 years ago

@nuno Quick question: Would it be ok to rename the module to "ti.collectionview" inside the require statement? Some advantages:

Really really no priority, but maybe for a future version.

nuno commented 7 years ago

Will be a non comp. version anyway, sure why not.

nuno commented 7 years ago

@hansemannn do you want share your test app.js with me, so can test what you tested?

hansemannn commented 7 years ago

@nuno Tested with the one attached to this PR so far (the new properties / events are untested so far). Also just tested on the device using run-on-main-thread and it just works. Are there common crashes related to certain functionalities? We should focus on crashes and errors in this PR and do following PR's for new features.

I could also think of an API interface that would allow other developers to bridge other UICollectionViewLayout-based modules that can be used together with this one. But all that should be done after this PR.

We also need better docs - either a table like in the Ti.WKWebView or a wiki-page like in Ti.Bluetooth. Thoughts?

nuno commented 7 years ago

1 - scrollend-event (works) 2 - scrollstart-event (works) 3 - LazyLoadingEnabled (works) 4 - setContentOffset (untested)

The table for the docs seems nice!

@hansemannn Im testing now against some current done app the use this module.

nuno commented 7 years ago

I forgot that "de.marcelpociot.CollectionView" name is already called in some community widgets and Modules, I just get some errors. That need to be addressed.

nl.fokkezb.pullToRefresh is one.

hansemannn commented 7 years ago

We need to update those when we launch 2.0.0. I also noticed that you pushed https://github.com/nuno/TiCollectionView/commit/4614034a43685f08c4ac9767a2c510ca80cdbb26 which updates a packaged version without having the commits merged in the repository, which will confuse people for sure. When do we want to integrate this?

nuno commented 7 years ago

@hansemannn, the 1.4.2 version is the version compatible with TIsdk 6.x and is your commit https://github.com/nuno/TiCollectionView/pull/66/commits/6481ea9dbe70ccc4051032cb4d88d3fce7217c05 that you bumped before the 2.0, it's not yet merged.

I checkout locally that commit and compiled the ios. I haven't changed nothing in the code manually or nothing like that.

Probably this should have his own PR or merged separately? I just wanted have one version comp. with ti 6.x before the 2.0 https://github.com/nuno/TiCollectionView/pull/66/commits/6481ea9dbe70ccc4051032cb4d88d3fce7217c05

Do you think that commit is danger or will break currents app? I test just on sample examples to be fair. the 1.4.1 on sdk 6.x not run and the 1.4.2 runs.

hansemannn commented 7 years ago

2.0 should still support Titanium 6.x, the breaking changes are the updated module ID and the removed context-menu library.

rlustemberg commented 7 years ago

I've just bumped into the same issue and happily found that it had been dealt here. Is there any priority for merging this PR? I see that the naming changing seems to be blocking progress. I'll pull from this branch in the meantime.

julien9999 commented 7 years ago

any dist ? thanks

hansemannn commented 7 years ago

It can be merged once @nuno approve's it. But as I also remove the 3rd-party context-menu (as I still think it should not be inside here but in an own module), I am not sure if this can ever be merged. You might want to grab a dist from the fork or the above PR-description for now.