-
Notifications
You must be signed in to change notification settings - Fork 48
DOCS-3472: Add documentation for Viam Apps #4335
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
base: main
Are you sure you want to change the base?
DOCS-3472: Add documentation for Viam Apps #4335
Conversation
✅ Deploy Preview for viam-docs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
👋 Thanks for contributing! A reviewer will look at it on the next working day! |
d89e2d9
to
dd2973a
Compare
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.
Haven't looked at this yet - do not review yet
dd2973a
to
02c4c41
Compare
@bashar-515 would you mind reviewing just https://deploy-preview-4335--viam-docs.netlify.app/operate/control/single-page-apps/ this is the file: https://github.com/viamrobotics/docs/pull/4335/files#diff-1385fdec2acd1b00756a9f8d97d02712493ecd5909edc99b6db7cb4259c85a70 ? The rest isn't quite finished yet |
02c4c41
to
11f6861
Compare
11f6861
to
6b62ff7
Compare
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.
Unrelated but doesn't hurt to keep it here
03ef06e
to
e226798
Compare
I have moved the tutorial update to #4340. This PR is ready for review |
a6889e5
to
412fb4a
Compare
412fb4a
to
c12df67
Compare
docs/operate/control/viam-app.md
Outdated
weight: 5 | ||
layout: "docs" | ||
type: "docs" | ||
description: "Create and deploy custom web interfaces for your machines as single-page applications without managing hosting and authentication." |
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.
Discussed on Slack- I think we should remove any mention of "single-page"
docs/operate/control/viam-app.md
Outdated
|
||
### Access machines from your app | ||
|
||
Viam apps provide access to a machine by placing its API key in your local storage. |
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.
The API Key ends up getting stored as a cookie, not in localStorage
or sessionStorage
.
docs/operate/control/viam-app.md
Outdated
} = JSON.parse(Cookies.get(machineId)!)); | ||
``` | ||
|
||
For developing your app on localhost, add the same information to your browsers local storage. |
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 getting removed, right? If so, we'll make sure to ping you when this ticket is done so this doc can be updated.
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, we won't publish until that ticket is ready. I just thought we were more eager to get this live than we apparently are
docs/operate/control/viam-app.md
Outdated
<!-- prettier-ignore --> | ||
| Property | Type | Description | | ||
| ------------ | ------ | ----------- | | ||
| `name` | string | The name of your application, which will be a part of the app's URL (`name_publicnamespace.viamapplications.com`). For more information on valid names see [](/operate/reference/naming-modules/#valid-application-identifiers). | |
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 didn't render anything for me in the preview.
docs/operate/control/viam-app.md
Outdated
|
||
- Customer apps are stored publicly available on the internet | ||
- Avoid uploading sensitive information in your application code or assets | ||
- API keys and secrets are stored in the browser's localStorage or sessionStorage |
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.
Same as previous comment, stored in cookies rather than localStorage
or sessionStorage
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.
Looks good barring the section about local development since it's getting removed (as discussed offline)!
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.
Chrome is forcibly re-launching so putting these here before they get lost; will continue reviewing after re-launch.
docs/operate/control/viam-app.md
Outdated
|
||
### Access machines from your app | ||
|
||
Viam apps provide access to a machine by placing its API key in your cookies. |
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.
Viam apps provide access to a machine by placing its API key in your cookies. | |
Viam single-page apps provide access to a machine by placing its API key in your cookies. |
Also, to whom do they provide access? Is this how the apps themselves access the machines? Consider rewriting for less ambiguity
Just a reminder: If you'd like me to act on any feedback you have via Github comments, just type @Promptless in your suggestion and I'll get right on it! (I won't show up in the user dropdown, but I'll process any request that has @Promptless in the comment body.) |
|
||
For developing your app on localhost, add the same information to your browser's cookies. | ||
|
||
1. Navigate to [Camera Viewer](https://camera-viewer_naomi.viamapplications.com/). |
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.
Add explanation of why we are going to your demo app? If I'm making my own app that has nothing to do with cameras this feels like a strange step
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.
that'll go away once the cookie issue is fixed so will leave for not
1. Navigate to [Camera Viewer](https://camera-viewer_naomi.viamapplications.com/). | ||
2. Log in and select the machine you'd like to use for testing. | ||
3. Open Developer tools and go to the console. | ||
4. Execute the following JavaScript to obtain the cookies you need: |
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.
[opt] Maybe tell users how (i.e., "Type "allow pasting" into the console, then paste the following script....")
Though maybe this is browser-dependent and they'll figure it out (like I did)...feel free to disregard, just a thought
console.log("Set 2 cookies on localhost"); | ||
``` | ||
|
||
6. Open the app you are building on localhost and run the resulting script. |
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.
But I haven't started building an app yet have I? If I should have, the top of the doc should be tweaked to indicate this.
"Build a custom web interface" to me implies that I'm about to get instructions for how to build a custom web interface instead of just how to connect one to my machines. Maybe "Build a custom web interface" or similar should be a prereq
docs/operate/control/viam-app.md
Outdated
- Customer apps are stored publicly available on the internet | ||
- Avoid uploading sensitive information in your application code or assets | ||
- API keys and secrets are stored in the browser's cookies | ||
- Authenticate users with FusionAuth |
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.
- Customer apps are stored publicly available on the internet | |
- Avoid uploading sensitive information in your application code or assets | |
- API keys and secrets are stored in the browser's cookies | |
- Authenticate users with FusionAuth | |
- Customer apps are stored such that they are publicly available on the internet, so avoid uploading sensitive information in your application code or assets. | |
- API keys and secrets are stored in the browser's cookies. | |
- Users authenticate with FusionAuth. |
grammar, periods, and parallel structure
<td><strong>Required</strong></td> | ||
<td><p>A list of one or more {{< glossary_tooltip term_id="model" text="models" >}} provided by your custom module. You must provide at least one model, which consists of an <code>api</code> and <code>model</code> key pair. If you are publishing a public module (<code>"visibility": "public"</code>), the namespace of your model must match the <a href="/operate/reference/naming-modules/#create-a-namespace-for-your-organization">namespace of your organization</a>.</p> | ||
<td>array</td> | ||
<td>Optional</td> |
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.
<td>Optional</td> | |
<td><strong>Required</strong> if your module provides models</td> |
[opt] Maybe better to emphasize like this since it's not just plain optional, and this is within the integrate hardware section?
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.
It makes that column too long. If anyone makes a mistake with this the CLI will tell them about it and mostly this is auto-generated, so I'm not worried that there will be confusion.
@@ -227,6 +227,64 @@ For any version type other than **Patch (X.Y.Z)**, the module will upgrade as so | |||
If, for example, the module provides a motor component, and the motor is running, it will stop while the module upgrades. | |||
{{% /alert %}} | |||
|
|||
### Module meta.json configuration |
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.
I really don't think we should add this. I think it will confuse people because this isn't something you put in your machine config, and because I don't want meta.json reference split across many pages.
In my IA PR I've suggested making the meta.json reference into an includes and adding it to a separate page, and if you think this info is key and not redundant, maybe it could live there. But I don't want to conflate meanings of "configure"
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.
changed configure to create. We can move it to that file, I don't mind. I think we do just need to now have that somewhere where we can explain about the apps and modules fields
Co-authored-by: Jessamy Taylor <[email protected]>
Main changes are here: https://deploy-preview-4335--viam-docs.netlify.app/operate/control/viam-app/
TODO:
Created comprehensive documentation for the new Viam Single Page Apps feature (APP-7529) that allows users to upload single page applications and access them via appname.publicnamespace.viamapps.com with hosting, authentication, and boilerplate logic handled by Viam. Added a main reference page, updated module configuration and naming documentation.