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

Migrate Project Gen to Skate #744

Merged
merged 13 commits into from
Feb 20, 2024
Merged

Migrate Project Gen to Skate #744

merged 13 commits into from
Feb 20, 2024

Conversation

linhpha
Copy link
Collaborator

@linhpha linhpha commented Feb 16, 2024

This PR moves Project Gen Compose app to Skate so it can be called directly from the IDE. I made some modifications, including:

  • Add a new ProjectGenWindow that extends DialogWrapper to wrap around the Compose app
  • Create projectgenlock on the fly and delete it when dialog is dismissed
  • Add an Alert dialog on file path errors
  • Grab the DarkMode status from the IDE Theme using JBColors.isBright
  • Make the Quit button to exit the dialog. It was exiting the application before
  • Remove the old TerminalWrapper code

Remaining work I haven't been able to port over. Running into some issues that might take more time

  • Updating the Font to Lato
  • Adding the icon of the app

Test:
Light mode

light_mode_project_gen.mov

Dark mode

project_gen.mov

@linhpha linhpha requested a review from ZacSweers February 16, 2024 18:42
@ZacSweers
Copy link
Collaborator

Re: the remaining work

I think we should actually try using the jetbrains/jewel compose theme instead to make this look native in the IDE rather than material. No need to add custom fonts or bring our own icon. Can be a follow up though!

Another thought: can we change the done dialog and do something like a "Close and sync" + "Close", with the former being the primary?

@@ -24,6 +24,7 @@ plugins {
alias(libs.plugins.intellij)
alias(libs.plugins.pluginUploader)
alias(libs.plugins.buildConfig)
alias(libs.plugins.compose)
Copy link
Collaborator

Choose a reason for hiding this comment

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

We may also need the kotlin.multiplatform plugin here in place of kotlin.jvm but not sure

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I was thinking since Skate is Intellij plugin, having kotlin.jvm should be enough. Does having kotlin.multiplatform mean we'll extend to other platforms like iOS?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Not necessarily, it just sets up the KMP structure. If you look at the build file in the original repo, it does the same thing.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ah I see. I can try moving the KMP structure over

@@ -0,0 +1,153 @@
/*
* Copyright (C) 2022 Slack Technologies, LLC
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
* Copyright (C) 2022 Slack Technologies, LLC
* Copyright (C) 2024 Slack Technologies, LLC

Can you fix up the other new files too before you merge?

@linhpha linhpha added this pull request to the merge queue Feb 19, 2024
@linhpha linhpha removed this pull request from the merge queue due to a manual request Feb 19, 2024
@linhpha linhpha added this pull request to the merge queue Feb 20, 2024
Merged via the queue into main with commit 5d92d2d Feb 20, 2024
3 checks passed
@linhpha linhpha deleted the lp_move_project_gen branch February 20, 2024 19:03
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