Skip to content

Conversation

@takahirom
Copy link
Contributor

Thanks for the many high quality samples!

I'm currently considering how to split files when building an app with Compose.
So I noticed that AppDrawer() used to be called by JetnewsApp() and now it's called by HomeScreen() and InterestsScreen(), and that AppDrawer() stays in JetnewsApp.kt. (aafa9ad#diff-098e0ebfb55fd1512e2e230dca256aaab9e14bdee57e7317f04dace7061d5803L61 )
Therefore, I moved it.
And there were previews such as PreviewJetnewsApp() and PreviewJetnewsAppDark(), but since it was a function for previewing AppDrawer, I changed it to PreviewAppDrawer().

@google-cla google-cla bot added the cla: yes label Nov 3, 2020
@nickbutcher nickbutcher requested a review from objcode November 3, 2020 11:43
@takahirom takahirom force-pushed the takahirom/move-app-drawer branch from 757b26e to 5118b65 Compare November 3, 2020 12:23
@takahirom
Copy link
Contributor Author

takahirom commented Nov 3, 2020

I added space to commit message.
[JetNews]" "Move AppDrawer Composable from JetnewsApp.kt to AppDrawer.kt

@takahirom takahirom force-pushed the takahirom/move-app-drawer branch from ac165fc to 16723f1 Compare November 6, 2020 01:12
@objcode
Copy link
Contributor

objcode commented Nov 6, 2020

@yrezgui can you include this with the alpha07 changes?

@takahirom takahirom force-pushed the takahirom/move-app-drawer branch from 16723f1 to 8e2e052 Compare November 12, 2020 01:06
@takahirom takahirom changed the title [JetNews]Move AppDrawer Composable from JetnewsApp.kt to AppDrawer.kt [JetNews] Move AppDrawer Composable from JetnewsApp.kt to AppDrawer.kt Nov 12, 2020
@takahirom
Copy link
Contributor Author

I rebased and fixed the import conflict. (with b0e28ed#diff-098e0ebfb55fd1512e2e230dca256aaab9e14bdee57e7317f04dace7061d5803L21 )

@florina-muntenescu florina-muntenescu deleted the branch android:dev_alpha07 November 12, 2020 09:47
@yrezgui
Copy link
Collaborator

yrezgui commented Nov 12, 2020

@takahirom, can you create another PR targeting the dev_alpha08 branch as we've merged the alpha07 samples?

@nickbutcher
Copy link
Contributor

*targeting dev_alpha08

@takahirom
Copy link
Contributor Author

Of course :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants