Skip to content

Add Kotlin to docs/new-architecture-library-intro - #3116

Merged
cortinico merged 4 commits into
react:mainfrom
mbharadwajfb:main
May 16, 2022
Merged

Add Kotlin to docs/new-architecture-library-intro#3116
cortinico merged 4 commits into
react:mainfrom
mbharadwajfb:main

Conversation

@mbharadwajfb

Copy link
Copy Markdown
Contributor

Added Kotlin example to docs/new-architecture-library-intr
Screenshot 2022-05-13 at 13 00 24
Screenshot 2022-05-13 at 13 00 19
o

@netlify

netlifyBot commented May 13, 2022

Copy link
Copy Markdown

Deploy Preview for react-native ready!

NameLink
🔨 Latest commitda53270
🔍 Latest deploy loghttps://app.netlify.com/sites/react-native/deploys/62826f9449b6020008f8e0d9
😎 Deploy Previewhttps://deploy-preview-3116--react-native.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@cortinico
cortinico self-requested a review May 13, 2022 15:26

```kotlin
fun receiveCommand(
view: ReactMapDrawerView?, commandId: String?, @Nullable args: ReadableArray?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
view: ReactMapDrawerView?, commandId: String?, @Nullable args: ReadableArray?
view: ReactMapDrawerView?, commandId: String?, args: ReadableArray?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Will do.

Comment threaddocs/new-architecture-library-intro.md
Comment on lines +574 to +576
if (args == null) {
break
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Have you tested this code? :)

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Fixing!

Comment on lines +577 to +578
val region: ReadableMap = args.getMap(0)
val durationMs: Int = args.getInt(1)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I would just wrap this inside an if (args != null) block

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Fixing

@cortinico

Copy link
Copy Markdown
Contributor

Thanks for sendign this over @mbharadwajfb 🙏 There are a couple of things to fix but we're good to go after this

@mbharadwajfb
mbharadwajfb marked this pull request as draft May 16, 2022 10:56
@mbharadwajfb

Copy link
Copy Markdown
ContributorAuthor

Code review fixes
Screenshot 2022-05-16 at 11 49 59

@mbharadwajfb
mbharadwajfb marked this pull request as ready for review May 16, 2022 11:20

@cortinicocortinico left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM. Just added a minor comment you forgot to add, but we can merge this 👍

Comment threaddocs/new-architecture-library-intro.md
@cortinico
cortinico merged commit 9ac35f0 into react:mainMay 16, 2022
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@mbharadwajfb@cortinico@facebook-github-bot