Skip to content
This repository has been archived by the owner on Jul 6, 2022. It is now read-only.

[WIP] SMS View #40

Closed
wants to merge 5 commits into from
Closed

[WIP] SMS View #40

wants to merge 5 commits into from

Conversation

bclymer
Copy link

@bclymer bclymer commented Dec 31, 2016

#23

Let's just say I'm not a design wizard, however, I got most of the backend working. It will automatically prefer you're primary device that supports SMS.

How to launch:
screenshot 2016-11-25 20 57 42

List of threads:
screenshot 2016-11-25 20 57 21

Conversation view:
screenshot 2016-11-25 20 57 32

I ended up being a fairly significant amount of refactoring. I've been working in Swift for a number of years professionally (although never for macOS, just iOS) but I hope you don't mind that I cleaned up and organized some stuff. I also updated to Swift 3 (to support Xcode 8).

@bclymer
Copy link
Author

bclymer commented Dec 31, 2016

What I ended up doing was just going to your new branch and manually copying in my files, and then ensuring that I didn't overwrite any of your changes from 0.2.

I would definitely give this a smoke test, and the UI still isn't complete. I spent some time on it, but I'm an iOS dev and it has become even more clear Apple cares a lot more about the development tools for iOS than they do for macOS, because I was on the struggle bus.

So if anybody else has a chance to take a look at the UI in this PR that'd be awesome.

@mirshko
Copy link

mirshko commented Jan 3, 2017

@bclymer Is the UI you're building my designs from the other ticket? #34

@bclymer
Copy link
Author

bclymer commented Jan 3, 2017

@mirshko it will be. I spent a decent amount of time on the UI which isn't reflected in the screenshots, but it took me a while to make it look even decent.

I'm hoping if you or @jariz is any better at macOS UI you could take a look. Everything is in individual xibs so it should be very easy to work on (I have a strong dislike for storyboards, but that's a topic for another time).

@jariz
Copy link
Owner

jariz commented Jan 3, 2017

I see you gave me write access to your branch, so I'll give it a shot!
Also, yea, I just threw everything in a single storyboard for the same reason the rest of the source is a mess (hint: it's because I'm lazy)

@mirshko
Copy link

mirshko commented Jan 3, 2017

I don't do swift dev so its all up to @jariz 😛

@jariz
Copy link
Owner

jariz commented Jan 15, 2017

@bclymer
Is stuff supposed to look like this?
I knew it was bad but can't even get it to display any messages 😞

EDIT: nevermind, I had encryption enabled and it doesn't support that yet. Will add error handling and stuff.

@bclymer
Copy link
Author

bclymer commented Jan 15, 2017

@jariz It definitely shouldn't look like that. If that's the initial screen I can only imagine that either it wasn't able to find a device on your account that supported SMS, or it wasn't able to fetch the ThreadPreview objects.

It's very possible the API is acting a bit differently for you than it was for me.

Oh -- I see your encryption comment now. I have no experience with that. If you do see any other bugs let me know.

@jariz jariz changed the title SMS View [WIP] SMS View Jan 17, 2017
@seanabraham
Copy link

Any updates here @jariz @bclymer? Anything I can do to help?

@bclymer
Copy link
Author

bclymer commented Feb 3, 2017

@seanabraham I haven't put any time into it recently. To be honest it's just not really a priority for me.

If you want to pull the branch and do some UI work it'd be appreciated. Pretty much all of the quality code is in the services, and you could scrap the UI completely and start from scratch if you wanted.

@jariz
Copy link
Owner

jariz commented Feb 3, 2017

@seanabraham I started a tiny little bit on the UI but haven't gotten that far yet, like brian says, if you file like you're up to the challenge, you're more than welcome.

@zachatrocity
Copy link

I messed with the UI a bit. What do you guys think:

@bclymer
Copy link
Author

bclymer commented Feb 6, 2017

@zachatrocity let's just say it's way better than it was when I was working on it :). For history for both @zachatrocity and @seanabraham , there are mocks in this PR

#34

@mirshko
Copy link

mirshko commented Feb 6, 2017

Deff getting closer, the sketch file is in that PR if you need to reference it for sizing. Deff needs more work to be closer to my designs.

Good start tho!

@zachatrocity
Copy link

@bclymer yeah I saw those mocks. Thanks! What do you think, should I be putting these changes as a PR into your SMS-Merge branch? or a new PR on @jariz's repo?

@mirshko yeah, I'm not super great with macOS development so its trial and error mostly. I'm not sure how to change the window color. Your mock ups are almost grey and I really like that. Next step though is the text field stuff.

@mirshko
Copy link

mirshko commented Feb 6, 2017

@zachatrocity they actually use the same vibrancy background that the settings panel uses. You will want to use the same setting that the panel uses for these. @jariz would know.

@bclymer
Copy link
Author

bclymer commented Oct 5, 2017

Closing the PR as I no longer have time to work on this, and I see it every time I go to my open pulls page and it makes me sad :(. I'll leave the branch around indefinitely though.

@bclymer bclymer closed this Oct 5, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants