RaspberryPiFoundation / editor-ui

Code Editor web component
https://editor-static.raspberrypi.org
Apache License 2.0
36 stars 9 forks source link

Editor image issues #1058

Closed conorriches closed 1 month ago

conorriches commented 1 month ago

Background

When working on the Block to Text editor, it was noticed images weren't loading which prevented progress through the steps.

Problems

Problem 1 - editor not loading on block to text branch

PROBLEM SOLVED - Scott identified env var and rebuilt

As a precursor to all of this, it was noted the editor doesn't load on the block-to-text-integration preview branch

This is due to a CORS error, it seems that the API images route won't allow the domain. Yesterday, I (Conor) updated the ALLOWED_ORIGINS env var to include this domain and rebuilt on staging, but the error persists

Problem 2 - images not loading on production

PROBLEM IDENTIFIED - CSV issue

Scott found these two URLs where images aren't loading

Problem 3 - images referenced by local name

LIKELY SOLVED - linked to problem 1

Locally, when everything is set up and running, and run Step 7 on block to text (local url), I noticed that the editor was asking for dog.webp which resolved to a localhost URL, instead of requesting this image from editor-api, which would return a rails active storage URL. There is a new environment variable (ASSETS_URL='https://staging-editor-api.raspberrypi.org/') but this didn't seem to do the trick.

For some reason this requests the image name instead of the image URL

Problem 4 - editor can constantly reload

Magda has an issue locally where the editor constantly reloads. @magdalenajadach mind filling this bit in on any symptoms?

conorriches commented 1 month ago

Working notes

I am not in Thurs or Fri, so I'm writing down where we are up to

Problem 1 - editor not loading on block to text branch

  1. Visit https://block-to-text-integration.projects-ui.pages.dev/en/exercises/example-exercise
  2. Note that web-component.js loads in dev tools
  3. Press "run" and note that p5.js and p5-shim.js fail to load
    • it seems that these URLs are https://staging-editor.raspberrypi.org/branches/output_only_wc/shims/processing/p5/p5-shim.js
    • instead of https://staging-editor-static.raspberrypi.org/.../shims/processing/p5/p5-shim.js
    • QUESTION: should we be on the output_only_wc branch? I recall hearing we should be using main and that the editor_output_wc branch isn't meant to be merged as the equivalent work has already been done in main

Things tried

grega commented 1 month ago

I'm still unclear where the ASSETS_URL env var is being set for this deployment: https://github.com/RaspberryPiFoundation/editor-ui/blob/00dbfc27d95434d24d30f8eab0ce7c344d58d644/src/components/Editor/Runners/PythonRunner/PythonRunner.jsx#L46-L47

But yes can confirm that I am seeing the result as being https://staging-editor.raspberrypi.org/... rather than https://staging-editor-static.raspberrypi.org/...

Despite these two values:

As with Conor's note above, I'm also confused with exactly what the output_only_wc branch is and where it's deployed to (and should it not be main?)

In order to unblock, I've done two things:

These are of course a bit heavy-handed and not meant to be long-term fixes by any means, but do allow https://block-to-text-integration.projects-ui.pages.dev/en/exercises/example-exercise to be run successfully eg.

Screenshot 2024-08-01 at 15 03 23

sra405 commented 1 month ago

Just wanted to pop a couple of quick responses here initially:

conorriches commented 1 month ago

Cheers both!

Just a note after working with @sra405 today (Scotts message above literally just came through while typing, so I'll try to be concise):

This should explain Problem 1 and Problem 3

sra405 commented 1 month ago

CSV issues should now be solved with https://github.com/RaspberryPiFoundation/editor-ui/pull/1062 - this appeared initially to be image issues however only impacted specific projects, it turned out these were loading in data or files that weren't properly attached to the DOM for Skulpt to pickup from the shadow DOM 🤦

grega commented 1 month ago

I've now deleted the temporary page rule and response modifier in Cloudflare (had seen they had already been disabled) 👍

conorriches commented 1 month ago

Closing as I believe this is now resolved, and the constant reloading issue doesn't appear to have persisted

conorriches commented 1 month ago

Thanks all for your input and help with this!