Skip to content
This repository was archived by the owner on Aug 1, 2019. It is now read-only.

changing to swift 4 apis and semantics#161

Open
mbalex99 wants to merge 4 commits into
eBay:masterfrom
mbalex99:swift4
Open

changing to swift 4 apis and semantics#161
mbalex99 wants to merge 4 commits into
eBay:masterfrom
mbalex99:swift4

Conversation

@mbalex99

@mbalex99 mbalex99 commented Oct 3, 2017

Copy link
Copy Markdown
Contributor

changing to swift 4 apis and semantics
The UI is a tiny bit wonky. I could use some help fixing stuff :-)
Update Thanks to @artanisdesign and @austinzheng for all the tiny fixes that totally eluded me!

So if you want Swift 4 today. You can install it like so:

pod 'NMessenger', :git => 'https://github.com/mbalex99/NMessenger', :branch => 'swift4'

@mbalex99 mbalex99 mentioned this pull request Oct 3, 2017
@mbalex99

mbalex99 commented Nov 11, 2017

Copy link
Copy Markdown
Contributor Author

Thanks so much @austinzheng for fixing ContentNodes an amazing. Job I would have never found it without his changes.

He was kind enough to give me the proper diffs to help me fix this.

This is ready for you guys to use it looks like! And certainly ready for a review!

@gorbat-o

Copy link
Copy Markdown

hey, it should be merge one day :)

@mbalex99

mbalex99 commented Nov 26, 2017 via email

Copy link
Copy Markdown
Contributor Author

@p1n0-0

p1n0-0 commented Nov 28, 2017

Copy link
Copy Markdown

I have verified that grouped messages are not displayed when sent. The first load works fine, but when you send several consecutive messages they do not get to show.

You can check it with the MessagesGroup example.

@artanisdesign

Copy link
Copy Markdown

Any idea how to fix that UI glitch? i guess this is happened because of the Texture (ASDK) update.. and i found this in the MessageGroup class

@artanisdesign

artanisdesign commented Nov 28, 2017

Copy link
Copy Markdown

Well, it seems adding
self.setNeedsLayout()
to line 201 solved the problem, PR in on @mbalex99 fork

@mbalex99

mbalex99 commented Nov 28, 2017

Copy link
Copy Markdown
Contributor Author

@artanisdesign I added it and that did the trick 🎊 . Committed and pushed!

@jacobokoenig

Copy link
Copy Markdown

Hey.. that pod says "Alamofire".. Mistake?

@mbalex99

mbalex99 commented Dec 5, 2017

Copy link
Copy Markdown
Contributor Author

@jacobo360 fixed. Thank you

@tyronegroves

Copy link
Copy Markdown

Will this be merged into the main code base?

@dodikk dodikk left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

So if you want Swift 4 today. You can install it like so:

Instructions for Carthage

github "mbalex99/NMessenger" "swift4"

Comment thread Cartfile
@@ -1 +1 @@
github "facebook/AsyncDisplayKit" "1.9.92"
github "texturegroup/texture"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Why no version here? The .podspec has it.

s.dependency "Texture", "2.5"

);
inputPaths = (
"${SRCROOT}/Pods/Target Support Files/Pods-MessageGroups/Pods-MessageGroups-frameworks.sh",
"${BUILT_PRODUCTS_DIR}/LoremIpsum/LoremIpsum.framework",

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Any idea "whose dependency is LoremIpsum.framework" ?

@dodikk

dodikk commented Jan 5, 2018

Copy link
Copy Markdown

Will this be merged into the main code base?

@tyronegroves , unfortunately, eBay takes actions really slowly. From time to time I start thinking they have abandoned this project ((
See this thread for details #149

P.S. Then one of them writes a comment. And again - the silence for months.

@dodikk

dodikk commented Jan 5, 2018

Copy link
Copy Markdown

Will this be merged into the main code base?

Actually, we can just switch to the "mbalex99/NMessenger" repo, merge all PRs from this one there, etc. In other words, an independent fork should be a way to go if there were a volonteer maintainer.

@tyronegroves

Copy link
Copy Markdown

Well I will try to find something else

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.

7 participants