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

feat: new onboarding step #3686

Merged
merged 11 commits into from
Sep 18, 2023
Merged

feat: new onboarding step #3686

merged 11 commits into from
Sep 18, 2023

Conversation

danil-pavlov
Copy link
Contributor

No description provided.

Copy link
Member

@koshachy koshachy left a comment

Choose a reason for hiding this comment

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

Some minor comments. Please resolve them before proceeding to the publishing.

In some sections sentences are a bit complicated, I'd like to divide them into two-three simple sentences.But overall it's a good tutorial, I really like it.

Great job!


![Common Kotlin, Kotlin/JVM, and Kotlin/Native](modules-structure.png)

### Writing common declarations
Copy link
Member

Choose a reason for hiding this comment

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

Why is Gerund here?
Isn't it better to use "Write" instead of "Writing"?

I'm pretty sure that we should use imperative verbs instead.

Writing the code only in common Kotlin has obvious limitations because it can't use any platform specifics.
Using interfaces and the [expect/actual](multiplatform-connect-to-apis.md) mechanism solves this.

### Adding platform-specific implementations
Copy link
Member

Choose a reason for hiding this comment

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

Add

2. Switch between the `androidMain` and the `iosMain` modules.
You'll see that they have different implementations of the same functionality for the Android and the iOS source sets:

```kotlin
Copy link
Member

Choose a reason for hiding this comment

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

Maybe using two different code blocks makes the different parts easier to read? 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Totally! we already started to do this in some later drafts as well

These are both declarative UI frameworks, and you'll see similarities in the UI implementations. In both cases,
you store the data in the `phrases` variable and later iterate over it to produce a list of `Text` items.

### The Android module
Copy link
Member

Choose a reason for hiding this comment

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

I expect here a verb. Like "update the android module" or something similar

<toc-element id="multiplatform-mobile-dependencies.md" toc-title="3. Add dependencies"/>
<toc-element id="multiplatform-mobile-upgrade-app.md" toc-title="4. Upgrade your app"/>
<toc-element id="multiplatform-mobile-wrap-up.md" toc-title="5. Wrap up your project"/>
<toc-element id="multiplatform-mobile-update-ui.md" toc-title="3. Update UI"/>
Copy link
Member

Choose a reason for hiding this comment

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

I'm thinking about expanding the toc-title to make it more readable.
In the nav tree the titles should be short (like "Share more logic"), but in the title of the content I'd like to see something like "Share more logic between iOS and Android").

What do you think about this?

Copy link
Contributor Author

@danil-pavlov danil-pavlov Sep 15, 2023

Choose a reason for hiding this comment

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

Makes sense, I got rid of the acronym and expanded the header for the 5th step

@JetBrains JetBrains deleted a comment from dsavvinov Sep 18, 2023
@danil-pavlov danil-pavlov merged commit 7f71d80 into master Sep 18, 2023
3 checks passed
@danil-pavlov danil-pavlov deleted the mpp-explore-project branch September 18, 2023 11:10
danil-pavlov added a commit that referenced this pull request Oct 17, 2023
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.

2 participants