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-9332] Add "centerOffset" to Map Annotations #4074

Closed
wants to merge 3 commits into from

Conversation

jjosef
Copy link
Contributor

@jjosef jjosef commented Apr 4, 2013

@negupta
Copy link
Contributor

negupta commented Apr 19, 2013

I could not find a signed CLA in place. Did you sign one?

- (void)setCenterOffset:(id)centeroffset
{
id current = [self valueForUndefinedKey:@"centerOffset"];
[self replaceValue:centeroffset forKey:@"centerOffset" notification:NO];
Copy link
Contributor

Choose a reason for hiding this comment

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

Since this ultimately becomes a CGPoint, it's better to be stored as an instance variable CGPoint instead of constantly converted multiple places.

@WhichKatieDid
Copy link
Contributor

@jjosef Please address comments and sign the Contributor's License Agreement (CLA) at https://appcelerator.echosign.com/public/hostedForm?formid=25LBTXJ55452R

@jjosef
Copy link
Contributor Author

jjosef commented Jun 5, 2013

@BlainHamon I have implemented using CGPoint not sure what else needs to be refactored. I could rewrite the entire function to be more comprehensive and concise, but I was trying to avoid changing too much, just needed this tweak for a client project and I thought other's would find it useful. If you think it can be improved, by all means, go ahead.

@vishalduggal
Copy link
Contributor

I think you misunderstood the review comments. I have set up a new PR based off of your PR.
PR #4433

Thank you for your contribution.
Closing this PR.

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

4 participants