-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
feat(android): added new methods in Ti.Calendar.Calendar module for bulk operations #14149
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you can add docs and an example test to test, this should work out.
android/modules/calendar/src/java/ti/modules/titanium/calendar/CalendarProxy.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just a small change as we have TiC properties for those parts
android/modules/calendar/src/java/ti/modules/titanium/calendar/CalendarUtils.java
Outdated
Show resolved
Hide resolved
android/modules/calendar/src/java/ti/modules/titanium/calendar/CalendarUtils.java
Outdated
Show resolved
Hide resolved
android/modules/calendar/src/java/ti/modules/titanium/calendar/CalendarUtils.java
Outdated
Show resolved
Hide resolved
will there be iOS parity? Guess it would be nice to have those methods there too |
@m1ga This PR came as a result of seeing super slow operations on Android with ANR logs sometimes. iOS seems already faster than current Android ones. It's definitely great to have iOS parity, but I suspect it might take a bit more time to come from my end :). |
Got it 👍 I didn't test it yet but sounds great that it is a performance boost! |
@m1ga Could you please review the format of docs added in yaml if they are fine? |
android/titanium/src/java/org/appcelerator/titanium/proxy/TiWindowProxy.java
Outdated
Show resolved
Hide resolved
… if not set already" This reverts commit e2c4bb9.
you can run |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
some minor code changes. Builds fine
android/modules/calendar/src/java/ti/modules/titanium/calendar/CalendarProxy.java
Outdated
Show resolved
Hide resolved
android/modules/calendar/src/java/ti/modules/titanium/calendar/CalendarProxy.java
Outdated
Show resolved
Hide resolved
android/modules/calendar/src/java/ti/modules/titanium/calendar/CalendarProxy.java
Outdated
Show resolved
Hide resolved
android/modules/calendar/src/java/ti/modules/titanium/calendar/CalendarProxy.java
Show resolved
Hide resolved
SDK builds fine. If you can create a simple test for the new methods I could quickly check the functionality too. Guess you have it running in your app already so I'm sure it will be fine |
@m1ga This should help you test the changes:
|
Function test: |
@prashantsaini1 Please remove the docs that are unrelated to this PR and make sure the action passes. You can also test api docs locally |
@m1ga As I was doing lots of lots of testing with 100/200+ events, my device feels a hang on syncing events from Google, so I do not see events added by either way now 😄 . Sometimes the event data is null in single update methods also, sometimes it's available. So I suspect it has something to do with syncing. |
yeah, I think if you delete it while it is syncing it will get that event from the cloud again. Happens sometimes when you do stuff quickly. But it looks fine with the 10 events: Bildschirmaufnahme_20241204_141118.webm |
…/CalendarProxy.java Co-authored-by: Michael Gangolf <[email protected]>
…/CalendarProxy.java Co-authored-by: Michael Gangolf <[email protected]>
…/CalendarProxy.java Co-authored-by: Michael Gangolf <[email protected]>
…ulk operations (#14149) * feat(android): added new methods in CalendarProxy for bulk operations * chore: use constant properties * fix: add missing properties of `scrolling` event * chore(android): add docs to new methods for Ti.Calendar.Calendar module * fix(android): set exitOnClose defaults to true on root window if not set already * Revert "fix(android): set exitOnClose defaults to true on root window if not set already" This reverts commit e2c4bb9. * fix: fix docs formatting * Update android/modules/calendar/src/java/ti/modules/titanium/calendar/CalendarProxy.java Co-authored-by: Michael Gangolf <[email protected]> * Update android/modules/calendar/src/java/ti/modules/titanium/calendar/CalendarProxy.java Co-authored-by: Michael Gangolf <[email protected]> * Update android/modules/calendar/src/java/ti/modules/titanium/calendar/CalendarProxy.java Co-authored-by: Michael Gangolf <[email protected]> * fix: fix docs --------- Co-authored-by: Michael Gangolf <[email protected]> Co-authored-by: Hans Knöchel <[email protected]>
This PR brings major improvements to deal with bulk operations in Ti.Calendar.Calendar module. Test results are below in this PR.
1. createEvents(args)
2. deleteEvents(ids)
<= ids.length
.== 1
can be applied to know if it was a successful deletion or not.3. getEventsById(ids)
deleteEvents()
above.<= ids.length
if some events are not created. Generally theid
property of created events can be used to compare with the passedids
array to know which events got successfully created.Test results to show approx time taken for 100 events operations.