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(import): github import wizard #6507

Merged
merged 28 commits into from
Oct 29, 2024
Merged

feat(import): github import wizard #6507

merged 28 commits into from
Oct 29, 2024

Conversation

liady
Copy link
Contributor

@liady liady commented Oct 9, 2024

This PR adds a Github import wizard, tracking the progress of importing a project from Github, showing errors or warnings. The design is still preliminary, but it is hidden behind a feature switch so that the logic can be reviewed and merges, in order not to create a long living PR.

Monosnap.screencast.2024-10-15.14-33-44.mp4

It supports error/warning states:
image

PR Includes:

  1. Adding three actions
    • SetImportWizardOpen - controlling the wizard UI
    • UpdateImportOperations - updating the results of individual import operations (desc. to follow).
    • UpdateProjectRequirements - updating the results of checking and fixing Utopia specific requirements
  2. Utilities to start/end tracking of import operations, their results and timings.
  3. Tracked import operations - tracking hooks are being called at appropriate places in the code.
    • Loading the code from Github
    • Parsing the files
    • Checking specific Utopia requirements
    • Loading dependencies
  4. Utopia checks:
    • storyboard file (created if not present)
    • utopia entry in package.json
    • project composition (JS/TS)
  5. Not tracked in this PR but will in subsequent ones (can be added modularily):
    • React version - currently a placeholder
    • Server side packages - mainly Node builtins
    • Style frameworks
  6. The wizard UI itself - showing progress and results (still preliminary)

Note for reviewers: there are a lot of changed files due to adding new actions and passing down the dispatch parameter, but the main logic is contained in the new services and the wizard React components.

Manual Tests:
I hereby swear that:

  • I opened a hydrogen project and it loaded
  • I could navigate to various routes in Play mode

Copy link
Contributor

github-actions bot commented Oct 9, 2024

Try me

Copy link

relativeci bot commented Oct 9, 2024

#14940 Bundle Size — 58.02MiB (+0.07%).

8fb5834(current) vs 916e8b9 master#14936(baseline)

Warning

Bundle contains 70 duplicate packages – View duplicate packages

Bundle metrics  Change 4 changes Regression 1 regression
                 Current
#14940
     Baseline
#14936
Regression  Initial JS 41.01MiB(+0.09%) 40.97MiB
No change  Initial CSS 0B 0B
Change  Cache Invalidation 18.29% 0%
No change  Chunks 20 20
No change  Assets 22 22
Change  Modules 4159(+0.17%) 4152
No change  Duplicate Modules 213 213
Change  Duplicate Code 27.32%(-0.04%) 27.33%
No change  Packages 477 477
No change  Duplicate Packages 70 70
Bundle size by type  Change 2 changes Regression 1 regression Improvement 1 improvement
                 Current
#14940
     Baseline
#14936
Regression  JS 58.02MiB (+0.07%) 57.98MiB
Improvement  HTML 7.37KiB (-0.25%) 7.39KiB

Bundle analysis reportBranch feat/github-import-wizardProject dashboard


Generated by RelativeCIDocumentationReport issue

import { RequirementResolutionResult } from '../../../core/shared/import/proejct-health-check/utopia-requirements-types'

export function OperationLine({ operation }: { operation: ImportOperation }) {
const operationRunningStatus = React.useMemo(() => {
Copy link
Contributor

Choose a reason for hiding this comment

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

the useMemo is not necessary here, because the return value is a string, which has value equality semantics by default

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was actually trying to save the internal comparisons from running every render if they don't need to (similar to what a computed field would have done)

@liady liady marked this pull request as ready for review October 20, 2024 03:04
@liady liady changed the title feat(import): github import wizard WIP feat(import): github import wizard Oct 20, 2024
@@ -1452,6 +1457,9 @@ export interface EditorState {
githubSettings: ProjectGithubSettings
imageDragSessionState: ImageDragSessionState
githubOperations: Array<GithubOperation>
importOperations: Array<ImportOperation>
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if these should be under some umbrella field rather than at the root of EditorState.

Copy link
Contributor Author

@liady liady Oct 28, 2024

Choose a reason for hiding this comment

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

interesting point! which field, for example? I followed the githubOperations line, which also describe a temporary chain of operations, very similar to the importOperations one

height: 12,
},
},
'.import-wizard-operation-children .operation-done [data-short-time=true]': {
Copy link
Contributor

@seanparsons seanparsons Oct 22, 2024

Choose a reason for hiding this comment

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

I feel a little uneasy about using these class names to hide things, it's a bit action at a distance to me.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good point! however the design is still temporary (and pending @lankaukk 's insights) - including whether or not to even have these kind of timings..
this is also the reason it's currently behind a (turned-off) flag - so we could merge the logic and iterate on the design later on..

operation: ImportOperation,
) {
if (operationRunningStatus === 'waiting') {
return 'gray'
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be using our color themes? As I would surmise that it will look a little weird in dark mode.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, definitely should come from the theme... this is also temporary design and definitely going to be changed in the design iteration

@liady liady merged commit 914b7bc into master Oct 29, 2024
16 checks passed
@liady liady deleted the feat/github-import-wizard branch October 29, 2024 12:06
liady added a commit that referenced this pull request Dec 13, 2024
This PR adds a Github import wizard, tracking the progress of importing
a project from Github, showing errors or warnings. The design is still
preliminary, but it is hidden behind a feature switch so that the logic
can be reviewed and merges, in order not to create a long living PR.
<video
src="https://github.com/user-attachments/assets/65c484b2-815a-4c07-8fb2-2346f06a466e"></video>

It supports error/warning states:
<img width="1362" alt="image"
src="https://github.com/user-attachments/assets/cc999476-f2d8-43bb-b7aa-4999fe15f589">

## PR Includes:
1. Adding three actions
    - `SetImportWizardOpen` - controlling the wizard UI
- `UpdateImportOperations` - updating the results of individual import
operations (desc. to follow).
- `UpdateProjectRequirements` - updating the results of checking and
fixing Utopia specific requirements
2. Utilities to start/end tracking of import operations, their results
and timings.
3. Tracked import operations - tracking hooks are being called at
appropriate places in the code.
    - Loading the code from Github
    - Parsing the files
    - Checking specific Utopia requirements
    - Loading dependencies
4. Utopia checks:
    - storyboard file (created if not present)
    - `utopia` entry in package.json
    - project composition (JS/TS)
5. Not tracked in this PR but will in subsequent ones (can be added
modularily):
    - React version - currently a placeholder
    - Server side packages - mainly Node builtins
    - Style frameworks
6. The wizard UI itself - showing progress and results (still
preliminary)

**Note for reviewers:** there are a lot of changed files due to adding
new actions and passing down the `dispatch` parameter, but the main
logic is contained in the new services and the wizard React components.

**Manual Tests:**
I hereby swear that:

- [X] I opened a hydrogen project and it loaded
- [X] I could navigate to various routes in Play mode
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.

5 participants