Skip to content
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

Add Workshop package #984

Open
wants to merge 23 commits into
base: main
Choose a base branch
from
Open

Add Workshop package #984

wants to merge 23 commits into from

Conversation

Velua
Copy link
Contributor

@Velua Velua commented Jan 13, 2025

This adds the Workshop.psi package with it's own UI which engages the registry.

Currently, whatever account the user is authenticated as is assumed to be the account of the actual app, after staged transactions is ready we'll later adopt it for a better multi-user experience.

The default header in sites for Content-Security-Policy in images has been updated to include :data so that image embedding from a string will work, subsequent changes will remove the idea of default headers and instead will be entirely configurable in the workshop app for developers to work with.

@Velua Velua marked this pull request as ready for review January 22, 2025 03:54
@Velua Velua changed the title Add registry Add Workshop package Jan 22, 2025
CMakeLists.txt Outdated Show resolved Hide resolved
services/user/Workshop/ui/README.md Outdated Show resolved Hide resolved
services/user/Workshop/ui/src/components/metadata-form.tsx Outdated Show resolved Hide resolved
services/user/Workshop/ui/src/components/nav.tsx Outdated Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this being used anymore?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed some stale config settings, me and @brandonfancher will go over the rest and figure out what makes sense and update all Vite projects.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah. I think we're going to need to do the opposite of what we've been doing. Instead of proxying from the vite dev server, we may want to proxy from the psibase config to the dev server for certain subdomains. Otherwise, our same-origin cross-subdomain OAuth-y stuff isn't going to work when trying to iterate quickly via the dev server, as it runs on another port.

@James-Mart James-Mart added the System app Related to system services and their apps/plugins label Jan 23, 2025
@Velua Velua requested a review from James-Mart January 24, 2025 03:41
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's remove this file and is reference in index.html

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is boilerplate that can be deleted, right?

<FormItem>
<FormLabel>App Name</FormLabel>
<FormControl>
<Input placeholder="MonsterApp" {...field} />
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd recommend a different default example here instead of a game. Maybe something related to voting or trading. Or a prediction market of some kind. 🤔

<FormItem>
<FormLabel>Terms of service</FormLabel>
<FormControl>
<Input placeholder="/terms-of-service" {...field} />
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was a little confused by the placeholder in these three fields. My initial intuition was that they were defaults that would actually get submitted if I didn't overwrite them. Instead, / was submitted for them.

I could see a couple ways of getting around that:

  • Make them required so the user has to explicitly input values, or
  • Make the placeholder value submit as the default value, unless overridden by the user

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This reminds me...should there be a dark/light toggle?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unused

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unused

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unused

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
System app Related to system services and their apps/plugins
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants