-
Notifications
You must be signed in to change notification settings - Fork 211
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
ugly glue for sharable kiosk code #6014
base: master
Are you sure you want to change the base?
Conversation
@@ -6,6 +6,15 @@ import { KioskState } from "./KioskState"; | |||
import configData from "../config.json"; | |||
import { getGameDetailsAsync } from "../BackendRequests" | |||
import { tickEvent } from "../browserUtils"; | |||
import { getSharedKioskData } from "../share"; | |||
|
|||
export interface KioskOpts { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is such a good change. I need to take advantage of interfaces and objects more often. Especially since we might be wanting to expand these options in the future. Thanks for doing this.
"kiosk", | ||
createProjectFiles("kiosk", kioskData) | ||
); | ||
const url = apiRoot + "/api/scripts"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So is this how we create all share links across targets? We send a POST request to this endpoint and the payload and header of the request determine what target is used and more?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, here's the entry point for where those get spun up in webapp https://github.com/microsoft/pxt/blob/master/webapp/src/app.tsx#L4106 & the actual requests are made in these two functions depending on if it's anonymous or persistent: https://github.com/microsoft/pxt/blob/master/webapp/src/app.tsx#L4106
|
||
const outputLink = `https://arcade.makecode.com/kiosk?shared=${sharePointer}`; | ||
|
||
alert(`send em to ${outputLink}`); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice. I'm guessing when kiosk starts using react-common we can just take advantage of the share dialog or a simplified version of it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Something like that, I just wanted to implement the smallest amount of ui in this that i could while still having an mvp to keep it simple / only spent about 20 minutes implementing / testing it (same reason this just comes from a button you have to click right now which isn't shippable, instead of hooking it into the gamepad movement flow); for now could just pop to a page with a qr code of it and link saying it's shareable as well if we wanted to add another state.
if (this.shareSrc) { | ||
const [id, ...filename] = this.shareSrc.split("/"); | ||
|
||
const kioskData = await getSharedKioskData(id, filename.join("/")); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this just for leaving us the option to send the shared kiosk entries to a different file/nested file? If I'm understanding everything correctly, is the filename kiosk.json
every time?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this one it's emitting as kiosk.json every time yes; I wrote it like this just to keep it simple, but we could choose to omit the filename when sharing it, and default to kiosk.json when filename is empty.
This mirrors our tutorial loading logic from shared projects / github projects -- defaulting to readme.md, but allowing a user to e.g. store multiple kiosks in one project if they wish to, just with a bit more specification involved on which file to read from.
}; | ||
files["pxt.json"] = JSON.stringify(config, null, 4); | ||
files["main.ts"] = " "; | ||
files["kiosk.json"] = JSON.stringify(kioskData, undefined, 4); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Asking for more understanding. Why are each of these files needed? Are these the files a share link is expected to serve so to speak?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
kiosk.json
and pxt.json
are actually necessary; pxt.json
is the project config and is required to be a valid makecode project (I'm guessing the post request would get rejected if we don't have a pxt.json in the blob but haven't tried; could just look at the backend code to confirm). kiosk.json
here is where we're storing the data we want to share (kiosk state), so that's important. I left an empty main.ts
just so there wouldn't be any weirdness if someone chose to open in the editor, not sure the behavior we would get from opening a shared project with no blocks/ts/py files in it off the top of my head and easiest just to include main.ts that is in almost every project rather than think about it (easy enough to remove from files
& config.files
to check though)
adds a button to create a share link with a kiosk in it:
which when clicked alerts out a link like https://arcade.makecode.com/kiosk?shared=_8oUCky7VwE91/kiosk.json ( for local testing would point at something like http://localhost:3000/static/kiosk?shared=_CCqJHuFeycUR/kiosk.json ), which when loaded shows up locked like this
This is not code to merge directly (well, could merge and not deploy, while tackling ui / ux layer, error handling and clean up in a separate pr if you want), I just decided to hack it together as a proof of concept for how easy it is to add in support. Probably need to poke around separately to make the state doesn't get persisted / user doesn't lose any locally added projects as well
(copied a good portion of the share.ts from richard font editor, which I believe was copied from asset editor, which was copied from pxtlib :) can dedup later)
we can change the format however we want; if we want to elide the /kiosk.json we could get away with saving it as
kiosk.data
or something and pulling in, just did an easy hack that semi matches github tutorials fetching logic. When this moves over to pxt / kiosk is transitioned to use pxtlib can dedup a bit and e.g. support github projects with kiosk data in them in the same way.