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

iOS 11 update #7

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

iOS 11 update #7

wants to merge 2 commits into from

Conversation

neoplastic
Copy link

Preliminarily resolves #6

This fixes some reds and modernises some of the code and app attributes.

This is far from complete.

  • Icons offset incorrectly on iPhone X's safe areas.
  • I feel like the opacity menu's selections are offset incorrectly
  • I don't exactly know what are the main issues in the app.

David Wong added 2 commits December 11, 2017 20:29
Modernized Podfile by wrapping pods in a Target.
Removed Testflight. Its hasn't been supported for years.
Fixed some changed OpenGL constant names.
@slarson
Copy link
Member

slarson commented Dec 12, 2017

@neoplastic Thanks for this! We are currently looking for additional testers to check this out and see if they can further improve on your fixes. Stay tuned!

Copy link

@chrisbrandow chrisbrandow left a comment

Choose a reason for hiding this comment

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

Hi. Quick background: I just read the recent article about the lego robot, and followed the links regarding the iOS app. I'm happy to help a bit. I'm an iOS developer as well as a chemist. I thought I'd start by just giving a cursory review of David's PR

The PR looks good given its purpose. Great Job! I got the project to build on a iPhone 8+ simulator. I have just two minor comments.

@@ -261,8 +261,8 @@ -(void) loadDrawGroupsInContext:(EAGLContext *)context
int bboffset = [[meshDictionary objectForKey:@"bboxes"] intValue];

if (bboffset > 0) {

int numBBoxen = [[meshDictionary objectForKey:@"names"] count];
NSArray *meshes = [meshDictionary objectForKey:@"names"];

Choose a reason for hiding this comment

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

if you're changing this line anyway, perhaps this would be clearer

NSArray *meshes = meshDictionary[@"names"];

@@ -82,7 +82,7 @@ - (void)viewDidLoad

-(void) viewDidAppear:(BOOL)animated
{
[TestFlight passCheckpoint:@"Search view displayed"];

Choose a reason for hiding this comment

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

if deleting the TestFlight line, might as well delete the method, particularly b/c it's not even calling super`.

Might as well delete the unused didReceiveMemoryWarning

@slarson
Copy link
Member

slarson commented Jan 30, 2018

@chrisbrandow Thanks for this! It is encouraging that it worked in a second person's hands. We're going to make a push on this in the coming weeks.

@neoplastic
Copy link
Author

Sorry, I got pretty busy and forgot to check in to this code review! Yeah, it was a quick and dirty job just to get this working, I did it on the train to work.

Thanks @chrisbrandow there's probably a lot more code that could be cleaned up and modernised along with the two you've found.

Honestly, I hadn't touched Obj-C for years now so it was nice making those connections again. If I get time again, I'll look at getting it laying out correctly with safe areas (iPhone X)

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.

Update app for iOS 11
3 participants