Initial support for OpenSCAD's customizer #477

Merged
Irev-Dev merged 5 commits from kurt/320-openscad-parms-demo-rebase into main 2021-08-27 12:47:09 +02:00
Irev-Dev commented 2021-08-26 22:53:40 +02:00 (Migrated from github.com)

This adds initial support for OpenSCAD's customizer 🙌

And also adds a select input for both SCAD and JSCAD from @hrgdavor.

I expect there to be a few quirks with this that will still need to be resolved, happy to merge with some quirks, just depending on how bad they are and we can fix later.

Preview links:
OpenSCAD
JSCAD

Resolves #320
Resolves #476

This adds initial support for OpenSCAD's customizer 🙌 And also adds a select input for both SCAD and JSCAD from @hrgdavor. I expect there to be a few quirks with this that will still need to be resolved, happy to merge with some quirks, just depending on how bad they are and we can fix later. Preview links: [OpenSCAD](https://6127ffbe0bdf1b000800901d--cadhubxyz.netlify.app/dev-ide/openscad#encoded_script_v2=eJydVFFvmzAQfudXnHgiEgESErUi4mFq1b10q7RU3UOVTQSOYNWxM9skWav99xkbEpK1WzVerPt833ef7s6EIRC25bRWCAVntXKcMIRvUBIhFeR8vUamgEioJRZQcqGzdozyrABFFEXwSIDgdhpDoxFIRd2BUbrhlPIdqArhboNsfvXhGlStuCAZTaBSaiOTMKSYCRbkWVHVy2D/8zkseC5DIyApKVDAjhQrVMYAq9dLg6gK1tk+gG1Ga3Rs4leNfsr2kEI8mYHmP06jxd+FCAORsVVf4UsT9zRGUTKKOh2Fm07sqOI08NyiKYwtLUqmPd4761/phqPQzU4hsjJDXT4KRlqqVdoQtuR72wFrhzwjjJz2IoXpzCTe415Bk9lUk0oQtnLskboV6sG4b+ZZcYpspY/LlqUbe2uQ1LU3bmPw0mgUKHNBNopwZmSWnOupMqc5HzI97iVtOqpEjbboA+Z6DWydGJBis2nS2Ro41qmPMAoiH8bByIc4mExh8Sdxck6cvEL0Ieq4t0RvtdSc3Bg1GnYM0qH67q78bKNm+O3s/dif+heLf/HbjmVLpGdazUbdGrynmtwx9OPkvhKI/jS5IVv0L5I5bpG9XcqO4U2rjeT9jvtW9IbX4n9cz02RE9Oucm0Fbmyr1nZpbcuebf1oHcwr7p08x8HsHDQv7AAf3s5pYvcUDqjd7yOtMXoaHVa0g/v712Htjp2Gky48ae6r4LE37+Cct1PfOiIrSC3TeObknHLh5al7nYmnj5wWyAQv3IHgKlP4XT9MURfoDZT+RUiqIe9xHPnRYpATkVP0ijSOBj2ZiivdpKd3CPCylKg8a6WLhm1YkLLUnWe5Zr44oL9jvaZRDdJTHI6jyB9O9a9uIH/UmdCQDnwDzJxfvwFfwhoo) [JSCAD](https://6127ffbe0bdf1b000800901d--cadhubxyz.netlify.app/dev-ide/jscad) Resolves #320 Resolves #476
Irev-Dev (Migrated from github.com) reviewed 2021-08-26 23:00:54 +02:00
@@ -22,3 +39,4 @@
)
return { consoleMessage, fullPath }
} catch (error) {
return { error, fullPath }
Irev-Dev (Migrated from github.com) commented 2021-08-26 23:00:54 +02:00

I had been storing the output logs from running CQ and OpenSCAD in s3 as metadata, and had planned to do the same thing with the customizer params, but realised this wasn't going to work since there is a tiny 2kb limit and that was already causing problems https://github.com/Irev-Dev/cadhub/issues/476

So now instead I'm concatenating the gzip file with and metadata (params and logs), its a little messy to split them apart in the browser but its working. /var/task/cadhub-concat-split has a unique string that I sue to split at.

I had been storing the output logs from running CQ and OpenSCAD in s3 as metadata, and had planned to do the same thing with the customizer params, but realised this wasn't going to work since there is a tiny 2kb limit and that was already causing problems https://github.com/Irev-Dev/cadhub/issues/476 So now instead I'm concatenating the gzip file with and metadata (params and logs), its a little messy to split them apart in the browser but its working. `/var/task/cadhub-concat-split` has a unique string that I sue to split at.
Irev-Dev (Migrated from github.com) reviewed 2021-08-26 23:02:14 +02:00
@@ -9,3 +6,1 @@
[CONSOLE_MESSAGE_KEY]: Buffer.from(consoleMessage, 'utf-8').toString(
'base64'
),
async function writeFiles(files = [], tempFile) {
Irev-Dev (Migrated from github.com) commented 2021-08-26 23:01:52 +02:00

Removed code to store things in s3 metadata.

Removed code to store things in s3 metadata.
Irev-Dev (Migrated from github.com) reviewed 2021-08-26 23:07:53 +02:00
@@ -13,3 +14,4 @@
RUN apt-get install -y openscad-nightly
# install node14, see comment at the to of node14source_setup.sh
ADD common/node14source_setup.sh /nodesource_setup.sh
Irev-Dev (Migrated from github.com) commented 2021-08-26 23:05:59 +02:00

Im replacing the release with the nightly build to keep things simple @t-paul, I didn't want to have to try and figure out to support multiple version right now.

Im replacing the release with the nightly build to keep things simple @t-paul, I didn't want to have to try and figure out to support multiple version right now.
Irev-Dev (Migrated from github.com) reviewed 2021-08-26 23:24:48 +02:00
@@ -106,0 +114,4 @@
} catch (e) {
console.log(json, e)
return {}
}
Irev-Dev (Migrated from github.com) commented 2021-08-26 23:24:48 +02:00

This is what splits and pulls the json meta off the concatenated file.

This is what splits and pulls the json meta off the concatenated file.
hrgdavor (Migrated from github.com) requested changes 2021-08-26 23:37:42 +02:00
hrgdavor (Migrated from github.com) left a comment

the choice input seems to work fine but color for values is a bit too close to the background and bit hard to read
image
it is likely a simple fix, and would be nice

Also I am not a fan of slider preview not respecting the step, it is a bit annoying for step:10... This one I am fine if you leave it like this, it is just a difference of opinion which is more natural to user, and can easily be changed in the future and is not breaking anything.

the choice input seems to work fine but color for values is a bit too close to the background and bit hard to read ![image](https://user-images.githubusercontent.com/2480762/131039168-f921d162-105a-47c4-81ff-f8afc2a31fe2.png) it is likely a simple fix, and would be nice Also I am not a fan of slider preview not respecting the step, it is a bit annoying for step:10... This one I am fine if you leave it like this, it is just a difference of opinion which is more natural to user, and can easily be changed in the future and is not breaking anything.
@@ -22,3 +39,4 @@
)
return { consoleMessage, fullPath }
} catch (error) {
return { error, fullPath }
hrgdavor (Migrated from github.com) commented 2021-08-26 23:26:09 +02:00

why not use https://www.npmjs.com/package/fflate
I started using it recently (to create 3mf export in my jscad prototype)

no temp file needed, lib is tiny, you can name the files when compressing, and decompressing and will likely be useful in the frontend in the future, so it will not be a bloat :)

... I would not stop this PR because of it, better not do too many things in a single PR. ... but maybe open an issue, so it is in the pipeline for later :)

why not use https://www.npmjs.com/package/fflate I started using it recently (to create 3mf export in my jscad prototype) no temp file needed, lib is tiny, you can name the files when compressing, and decompressing and will likely be useful in the frontend in the future, so it will not be a bloat :) ... I would not stop this PR because of it, better not do too many things in a single PR. ... but maybe open an issue, so it is in the pipeline for later :)
hrgdavor (Migrated from github.com) reviewed 2021-08-26 23:40:49 +02:00
@@ -106,0 +114,4 @@
} catch (e) {
console.log(json, e)
return {}
}
hrgdavor (Migrated from github.com) commented 2021-08-26 23:40:49 +02:00

it is ok for now, you control both sides, so It can be improved later

it is ok for now, you control both sides, so It can be improved later
hrgdavor commented 2021-08-26 23:47:28 +02:00 (Migrated from github.com)

image
last input does not respect the initial value for some reason ...

image
same happens in jscad

![image](https://user-images.githubusercontent.com/2480762/131040395-d75bee1d-41da-4f63-91ab-67ec75fceb27.png) last input does not respect the initial value for some reason ... ![image](https://user-images.githubusercontent.com/2480762/131040500-5ff82e90-f0f0-4146-9a10-aa0a4df7e200.png) same happens in jscad
t-paul (Migrated from github.com) reviewed 2021-08-27 00:23:42 +02:00
@@ -13,3 +14,4 @@
RUN apt-get install -y openscad-nightly
# install node14, see comment at the to of node14source_setup.sh
ADD common/node14source_setup.sh /nodesource_setup.sh
t-paul (Migrated from github.com) commented 2021-08-27 00:23:42 +02:00

Yep, keeping it simpler for now seems a good idea. This version is actively used by people (including myself) so it should be reasonably stable.

Yep, keeping it simpler for now seems a good idea. This version is actively used by people (including myself) so it should be reasonably stable.
Irev-Dev commented 2021-08-27 09:05:43 +02:00 (Migrated from github.com)

Hrg also said to me separately (listing here so I don't loose track):

bgcolor for choice and last input not respecting initial value are important (relates to #472)
my comment on step during preview, not so much, u can ignore taht one
zip also, can be done in another iteration

Hrg also said to me separately (listing here so I don't loose track): bgcolor for choice and last input not respecting initial value are important (relates to #472) my comment on step during preview, not so much, u can ignore taht one zip also, can be done in another iteration
Irev-Dev (Migrated from github.com) reviewed 2021-08-27 09:10:44 +02:00
@@ -22,3 +39,4 @@
)
return { consoleMessage, fullPath }
} catch (error) {
return { error, fullPath }
Irev-Dev (Migrated from github.com) commented 2021-08-27 09:10:44 +02:00

I was trying to stick to native browser ungziping things when the request arrives with the correct headers. But something like this might be better

I am not too concerned about finding an optimum solution right now since the next thing on my todo list is investigate performance, and I have a feeling that a lot of backend things might change in the process. Thanks for the lib recommendation.

I was trying to stick to native browser ungziping things when the request arrives with the correct headers. But something like this might be better I am not too concerned about finding an optimum solution right now since the next thing on my todo list is investigate performance, and I have a feeling that a lot of backend things might change in the process. Thanks for the lib recommendation.
Irev-Dev commented 2021-08-27 09:13:49 +02:00 (Migrated from github.com)

last input does not respect the initial value for some reason ...

👍 I'll look into this,

the choice input seems to work fine but color for values is a bit too close to the background and bit hard to read

This doesn't happen for me, I'm guessing because this is using the browser native select so default styles vary. Its a dark background for me, Should be an easy fix though.

> last input does not respect the initial value for some reason ... 👍 I'll look into this, > the choice input seems to work fine but color for values is a bit too close to the background and bit hard to read This doesn't happen for me, I'm guessing because this is using the browser native select so default styles vary. Its a dark background for me, Should be an easy fix though.
hrgdavor (Migrated from github.com) approved these changes 2021-08-27 12:41:45 +02:00
hrgdavor (Migrated from github.com) left a comment

Confirmed,

  • select box looks ok
  • input initial value is now respected also for the last item

This is now ready to merge as far as I am concerned, my other comments can be considered later on

Confirmed, - select box looks ok - input initial value is now respected also for the last item This is now ready to merge as far as I am concerned, my other comments can be considered later on
Sign in to join this conversation.
No Label
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: h3n3/cadhub#477