search

chinchang / web-maker

A blazing fast & offline frontend playground
https://webmaker.app
MIT License
2.55k stars 314 forks source link

(v5) fix detached preview not working in Files mode and some nits #502

Closed arnabsen1729 closed 2 years ago

arnabsen1729 commented 2 years ago

Fixes: #443

1. Detached Preview Fix

The iframe in the case of Files Mode has src set to http://localhost:7888/index.html. When we click on detached preview, it opens a new window and checks for two cases, one for the normal mode and the other for the Files Mode. In the latter it had the condition;

    if (e.data && e.data.url && e.data.url.match(/preview\.html/)) {
        document.querySelector('iframe').src = e.data.url;
    }

but as we mentioned before the src ends with index.html. So replacing the preview with index fixed the issue.

https://user-images.githubusercontent.com/51032928/158008382-549db3ae-6390-4ad0-906b-eb03845a8164.mp4

2. Small fix in src/components/Modal.jsx.

Currently, in the v5 branch we see this error in the "files mode".

image

The fix was to remove the this from the this.onKeyDownHandler. We have shifted to functional components in Modal.jsx in v5.

3. Added a new dev dependency concurrently

When we are dealing with Files Mode in development. We need to have a separate server serving the preview.html. We do that using gulp. Running gulp start-preview-server achieves this:

image

So we have to keep this running in the background and then run the npm run dev in another terminal which is a lot of hassle. For that, we use this tool concurrently.

Now, we can just run npm run start and it will start the gulp preview server and the application together. We are using the flag --kill-others so that it kills other processes if one exits (when we press Ctrl + C) or dies.

image

chinchang commented 2 years ago

Wohooo!!! Amazing fixes! Love the speed and quality 😃

One thing, we should try not to club different independent changes into one PR. Reason:

  1. It makes it difficult for reviewer because there is no single context of the PR now.
  2. Everything goes together, or doesn't go together. If they are all separate PRs, each one go live as they get reviewed without blocking one another.
arnabsen1729 commented 2 years ago

Yeah, makes more sense to keep them independent. Will keep it in mind next time.