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

MOD-2418/2450 TI.Map iOS: Support rotation on annotations #293

Merged
merged 4 commits into from Sep 4, 2020

Conversation

anil-shukla-axway
Copy link
Contributor

@anil-shukla-axway anil-shukla-axway commented Apr 21, 2020

https://jira.appcelerator.org/browse/MOD-2418
https://jira.appcelerator.org/browse/MOD-2450

Commit 1dd8d9f is just to revert changes of PR #297, which will reopen ticket MOD-2103 as the change caused a regression.

@build
Copy link

build commented Apr 21, 2020

Fails
🚫 version bump was Minor in ios/manifest but Patch in package.json
Warnings
⚠️ SDK version declared in Jenkinsfile (9.0.0.v20200207060625) does not match iOS' titanium.xcconfig value (9.0.0.GA)
Messages
📖

💾 Here are the artifacts produced:

📖

✅ All tests are passing
Nice one! All 119 tests are passing.

📖 🎉 - congrats on your new release

Generated by 🚫 dangerJS against 3dcbbc1

Copy link
Contributor

@vikas-goyal-axway vikas-goyal-axway left a comment

Choose a reason for hiding this comment

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

@anil-shukla-axway , here we don't have any function to change the rotation of specific annotation and animate the location update. As discussed please add these functions.

@fasihul-haque
Copy link

fasihul-haque commented Apr 21, 2020

Test cases are missing, please add test case to verify the feature as mentioned on ticket. Also rebase with master branch, as current branch looks out dated.

@vikas-goyal-axway vikas-goyal-axway changed the title MOD-2418/2450TI.Map iOS: Support rotation on annotations MOD-2418/2450 TI.Map iOS: Support rotation on annotations Apr 24, 2020
@vijaysingh-axway
Copy link
Contributor

vijaysingh-axway commented May 4, 2020

@anil-shukla-axway @vikas-goyal-axway @fasihul-axway I think we should introduce 2 new properties on annotation -

  1. For rotation of annotation - Ti.Map.annotation.rotate(angle) . We want something similar to android's rotation of marker https://developers.google.com/maps/documentation/android-sdk/marker#rotate_a_marker .
  2. For animation of annotation- Ti.Map.annotation.animate(newLocation)

See following example -
Test Case -
`var Map = require('ti.map');
var win = Ti.UI.createWindow({
backgroundColor: 'white',
title: 'Rotate'
});

var annotation = null;

var map = Map.createView({
region: {
latitude: 45,
longitude: -76,
latitudeDelta: 0.08,
longitudeDelta: 0.08
},
bottom: 100
});
win.add(map);

var animateButton = Ti.UI.createButton({
title: "Animate annotation to new location",
bottom: 5
});
win.add(animateButton);
animateButton.addEventListener('click', function() {
var newCoordinate = [45.05,-76.05];
annotation.animate(newCoordinate);
});

var rotateButton = Ti.UI.createButton({
title: "Rotate annotation to 90 degree",
bottom: 50
});
win.add(rotateButton);

rotateButton.addEventListener('click', function() {
annotation.rotate(90);
});

win.open();

createAnnotations();

function createAnnotations() {
annotation = Map.createAnnotation({
latitude: 45,
longitude: -76,
title: "annotation "
});
map.addAnnotation(annotation);
}`

Let me know if you have any question.

@anil-shukla-axway
Copy link
Contributor Author

@anil-shukla-axway @vikas-goyal-axway @fasihul-axway I think we should introduce 2 new properties on annotation -

  1. For rotation of annotation - Ti.Map.annotation.rotate(angle) . We want something similar to android's rotation of marker https://developers.google.com/maps/documentation/android-sdk/marker#rotate_a_marker .
  2. For animation of annotation- Ti.Map.annotation.animate(newLocation)

See following example -
Test Case -
`var Map = require('ti.map');
var win = Ti.UI.createWindow({
backgroundColor: 'white',
title: 'Rotate'
});

var annotation = null;

var map = Map.createView({
region: {
latitude: 45,
longitude: -76,
latitudeDelta: 0.08,
longitudeDelta: 0.08
},
bottom: 100
});
win.add(map);

var animateButton = Ti.UI.createButton({
title: "Animate annotation to new location",
bottom: 5
});
win.add(animateButton);
animateButton.addEventListener('click', function() {
var newCoordinate = [45.05,-76.05];
annotation.animate(newCoordinate);
});

var rotateButton = Ti.UI.createButton({
title: "Rotate annotation to 90 degree",
bottom: 50
});
win.add(rotateButton);

rotateButton.addEventListener('click', function() {
annotation.rotate(90);
});

win.open();

createAnnotations();

function createAnnotations() {
annotation = Map.createAnnotation({
latitude: 45,
longitude: -76,
title: "annotation "
});
map.addAnnotation(annotation);
}`

Let me know if you have any question.

I have followed the suggestions(Teams chat) from you.. I found that the methods of class TiMapAnnotationProxy are used to set properties of annotations below is example-

var anno4 = Map.createAnnotation({
latitude: -33.86365,
longitude: 151.22689,
title: 'Anno4',
subtitle: 'This is anno4',
draggable: true
});

so If I create one method - (void)animate:(id)args; this has be like setting the animate([annotation, destLatLong]) at the time of annotation creation.

Also the below method --

  • (void)setAnimate:(id)arg
    {
    ENSURE_SINGLE_ARG(arg, NSArray);
    TiMapAnnotationProxy *newAnnotation = [arg objectAtIndex:0];
    NSArray *newLoc = [arg objectAtIndex:1];
    CLLocationCoordinate2D newLocation;
    newLocation.latitude = [[newLoc objectAtIndex:0] floatValue];
    newLocation.longitude = [[newLoc objectAtIndex:1] floatValue];

    [(TiMapView *)[self view] animateAnnotation:newAnnotation withLocation:newLocation];
    }

here user has to provide annotation and destination lat long so on behalf of which an angle is being calculated and also the annotation is being animated.
So do we also have to provide a separate method to rotate the angle? as its been done by this below method already ..

  • (void)animateAnnotation:(TiMapAnnotationProxy *)newAnnotation withLocation:(CLLocationCoordinate2D)newLocation
    {
    userOldLocation.latitude = newAnnotation.coordinate.latitude;
    userOldLocation.longitude = newAnnotation.coordinate.longitude;
    userNewLocation.latitude = newLocation.latitude;
    userNewLocation.longitude = newLocation.longitude;
    float getAngle = [self angleFromCoordinate:userOldLocation toCoordinate:userNewLocation];

    [UIView animateWithDuration:2
    animations:^{
    newAnnotation.coordinate = userNewLocation;
    MKAnnotationView *annotationView = (MKAnnotationView *)[self.map viewForAnnotation:newAnnotation];
    annotationView.transform = CGAffineTransformMakeRotation(getAngle);
    }];
    }

Copy link
Contributor

@vijaysingh-axway vijaysingh-axway left a comment

Choose a reason for hiding this comment

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

@anil-shukla-axway Everything looks good. There are few polishing changes required. Please address that. After that we will be good with this PR.

ios/Classes/TiMapView.m Outdated Show resolved Hide resolved
ios/Classes/TiMapAnnotationProxy.h Outdated Show resolved Hide resolved
ios/Classes/TiMapAnnotationProxy.m Outdated Show resolved Hide resolved
ios/Classes/TiMapView.m Outdated Show resolved Hide resolved
ios/example/tests/rotateAnnotation.js Outdated Show resolved Hide resolved
ios/example/tests/annotations.js Outdated Show resolved Hide resolved
@anil-shukla-axway anil-shukla-axway added the WIP Work in progress label May 28, 2020
@anil-shukla-axway
Copy link
Contributor Author

0c66736 will open ticket MOD-2103 as the previous changes were causing a regression.

@anil-shukla-axway anil-shukla-axway added WIP Work in progress and removed WIP Work in progress labels Jun 1, 2020
@fasihul-haque
Copy link

@anil-shukla-axway, there are too many commit and some are useless and may cause confusion if someone dig and want to see history of changes. Please refactor and use specific commit with appropriate message. You have used feat in almost all commit which is for new feature, please use appropriate key.

@anil-shukla-axway
Copy link
Contributor Author

@anil-shukla-axway, there are too many commit and some are useless and may cause confusion if someone dig and want to see history of changes. Please refactor and use specific commit with appropriate message. You have used feat in almost all commit which is for new feature, please use appropriate key.

Made the required changes, please verify.

Copy link

@fasihul-haque fasihul-haque left a comment

Choose a reason for hiding this comment

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

Changes looks good 👍

Copy link
Contributor

@vijaysingh-axway vijaysingh-axway left a comment

Choose a reason for hiding this comment

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

@anil-shukla-axway Changes looks good. Please document these APIs.

@anil-shukla-axway anil-shukla-axway added the WIP Work in progress label Jun 5, 2020
Copy link
Contributor

@vijaysingh-axway vijaysingh-axway left a comment

Choose a reason for hiding this comment

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

CR passed .

@fasihul-haque fasihul-haque removed the WIP Work in progress label Jun 5, 2020
- name: newLocation
summary: latitude and longitude where annotation will animate.
type: Array<Number>
platforms: [iphone, ipad]
Copy link
Contributor

Choose a reason for hiding this comment

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

A since property needs to be set on new APIs. This should be set to the SDK version we're expecting to ship the changes in.

Copy link
Contributor

@ewanharris ewanharris left a comment

Choose a reason for hiding this comment

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

Doc changes look good!

@ssjsamir ssjsamir self-requested a review September 3, 2020 13:32
Copy link
Contributor

@ssjsamir ssjsamir left a comment

Choose a reason for hiding this comment

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

FR Passed, Able to rotate and animate annotations, tested using the ti.map example app.

Test environment

MacOS Big Sur: 11.0 Beta 5
Xcode: 12.0 Beta 6
Java Version: 1.8.0_242
Android NDK: 21.3.6528147
Node.js: 12.18.1
""NPM":"5.0.0","CLI":"8.1.0-master.11""
iphone 8 Sim (14.0 Beta)

@ssjsamir
Copy link
Contributor

ssjsamir commented Sep 3, 2020

@anil-shukla-axway Could you make updates to the package.json and manifest files so we can publish a new version of ti.map.

@satinder-singh-bamrah
Copy link

@anil-shukla-axway, Also please update the since property for YML change for newly added methods to 9.2.0 as this will be part of 9.2.0 release. Correct me @ssjsamir if wrong.

@ssjsamir ssjsamir merged commit bb9bdf2 into tidev:master Sep 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants