Open AWolf81 opened 5 years ago
Merging #324 into master will increase coverage by
0.55%
. The diff coverage is29.61%
.
@@ Coverage Diff @@
## master #324 +/- ##
==========================================
+ Coverage 20.91% 21.47% +0.55%
==========================================
Files 246 255 +9
Lines 3896 4094 +198
Branches 389 413 +24
==========================================
+ Hits 815 879 +64
- Misses 2802 2922 +120
- Partials 279 293 +14
Impacted Files | Coverage Δ | |
---|---|---|
src/components/Modal/Modal.js | 0% <ø> (ø) |
:arrow_up: |
src/config/app.js | 100% <ø> (ø) |
:arrow_up: |
src/store/index.js | 0% <0%> (ø) |
:arrow_up: |
src/components/ModalHeader/ModalHeader.js | 0% <0%> (ø) |
:arrow_up: |
src/components/DisabledText/index.js | 0% <0%> (ø) |
|
src/components/TokenInput/TokenInput.js | 0% <0%> (ø) |
|
src/components/ExportToCodesandbox/index.js | 0% <0%> (ø) |
|
src/sagas/index.js | 0% <0%> (ø) |
:arrow_up: |
src/components/FormField/FormField.js | 0% <0%> (ø) |
:arrow_up: |
src/components/DisabledText/DisabledText.js | 0% <0%> (ø) |
|
... and 22 more |
I have not dived into the code yet to review but I am trying to test out the flow in Guppy and I'm getting ENOENT when trying to export... is that functionality still WIP? It also seems to trigger when I hit the Logout button as well, which is strange.
OK, I think this is ready for a review. I have two smaller things open but it would be great to get a feedback to it.
The UI is not perfect yet. If you're having any ideas what should be improved please let me know. What do you think? Is it OK to move the save button to the header of the modal (see screenshot above)? I've moved it to save some space in the modal content - otherwise it wasn't fitting. Sure adding a scroll would help but I don't like to have the save button of screen. Or maybe we could add the save bottom to a bottom bar that's permanently visible and use a scrollbar.
@melanieseltzer where do you get that message? Is this happening after triggering export action?
@AWolf81 Here's a recording...
The export never completes and the button stays spinning. And as you can see it's also triggering about clicking the Logout button.
Seems like codesandbox cli
is missing. Have you installed it with a yarn
call?
If it's installed and still happening I think that could be a path issue.
@AWolf81 🤦♀️ That was it, whoops.
I'm not too familiar with Codesandbox and the CLI, do you know why I'm getting an infinite loop of tokens? Every time it finds error with the token and prompts me to get a new one. Am I doing something wrong?
@melanieseltzer no problem. :-)
That's weird. Maybe sendCommandToProcess
is not properly working as I'm seeing many characters from the token in the terminal log. I've had a similar behaviour before I've added the currentStep
counter - maybe this is not working as expected.
The Codesandbox shouldn't loop. The normal CLI flow is like following:
codesandbox .
The token is stored at the CLI location that's why I've added a logout button which will call codesandbox logout
.
@melanieseltzer I had the same behaviour like you. Seems like a node version issue with codesandbox cli. What node version do you use? If it's >10 there is an issue as the CLI doesn't open the browser window & it's stuck before opening the browser. v8 is working.
Issue on Codesandbox Cli already tracking it. Seems like it is related to opn package - I'll have a look into that package.
My first shot at getting this running resulted in getting stuck with a loading spinner on the Export to Codesandbox
button and no sandbox created on their end. Error message is spawn codesandbox ENOENT
, running on Ubuntu 18.04. I'll update this comment as I figure things out.
If the looping issue is due to codesandbox cli
, then we can potentially fork our own version and just set a fallback if something goes wrong, maybe log the intended URL to the console and have the user ctrl+click on it instead. I'll dig into this further and see if I can replicate it.
Related Issue:
323
Summary: Exporting to Codesandbox working in branch
export-to-codesandbox
. Just the mentioned issue with the image not fixed yet in Codesandbox. I have to check what's wrong with it.I'm creating this because I'd like to get a first feedback if this is going into the right direction. If it's OK then I'll start to refactor the business logic from the component into
export-codesandbox.saga
.Save button styling at the end of the dialog is a bit messed. I have to check why this happened.
I think the export flow is OK like it is in the branch. The Codesand token is saved in app-settings.reducer so it's available for every project.
TODOs
export-codesandbox.saga
logout
test$FlowFixMe
added.codesandbox
- shouldn't happend but we need to show an error message likeExport failed. Please check browser console for more details.
& also finish exporting.Screenshots/GIFs: