Feat: Make automation token UI conform to figma #5208
Conversation
vbustamante
commented
Jan 6, 2025
vbustamante
commented
Signed-off-by: Victor Bustamante <[email protected]>
Dependency Review✅ No vulnerabilities or OpenSSF Scorecard issues found.OpenSSF Scorecard
Scanned Files |
| @@ -55,6 +68,16 @@ const revoke = useAsyncState( | |||
| { immediate: false }, | |||
| ); | |||
|
|
|||
| const createdAt = computed(() => | |||
| new Date(props.authToken.createdAt).toLocaleString(), | |||
There was a problem hiding this comment.
Personal preference, but maybe something we should consider more broadly: for small snippets we don't reuse, it's probably better not to put this stuff in <script setup>, just so we can see the dependency flow more easily (not have to travel up and down between and <script setup> to understand what something is).
Vue will make the {{ new Date(props.authToken.createdAt).toLocaleString() }} completely reactive (anything in the template is), so they'll have the same effect.
(Not blocking in the least. Intended for discussion.)
| /> | ||
| </tbody> | ||
| </table> | ||
| </Stack> | ||
| </div> | ||
| </Stack> | ||
| </div> | ||
| <Modal ref="tokenDisplayModalRef" size="lg" title="Token Generated"> | ||
| <ErrorMessage |
There was a problem hiding this comment.
Nice touch making it red until you copy!
... though is there a SuccessMessage or something one could use for the happy case?
There was a problem hiding this comment.
I think the move here would be to have make this into a statusMessage or messageBox component eventually, since we don't have anything else
| import { useWorkspacesStore, WorkspaceId } from "@/store/workspaces.store"; | ||
| import WorkspacePageHeader from "@/components/WorkspacePageHeader.vue"; | ||
| import { useAuthTokensApi } from "@/store/authTokens.store"; | ||
| import AuthTokenListItem from "@/components/AuthTokenListItem.vue"; | ||
|
|
||
| useHead({ title: "Automation Tokens" }); |
There was a problem hiding this comment.
Should I be doing this on all pages?
There was a problem hiding this comment.
@jkeiser This can be done on the router too, but on the auth portal we've been using this pattern. I'd say useHead on the auth portal, the router title on the webapp
| if (createAuthToken.state.value) { | ||
| await navigator.clipboard?.writeText(createAuthToken.state.value); | ||
| } | ||
| tokenCopied.value = true; |
There was a problem hiding this comment.
It seems unlikely to trigger as a bug, but this should probably be inside that if
| @@ -175,8 +199,14 @@ const createAuthToken = useAsyncState( | |||
| /** Name of token to create */ | |||
| const createAuthTokenName = ref(""); | |||
|
|
|||
| /** Token modal */ | |||
| const tokenDisplayModalRef = ref<InstanceType<typeof Modal> | null>(null); | |||
There was a problem hiding this comment.
| const tokenDisplayModalRef = ref<InstanceType<typeof Modal> | null>(null); | |
| const tokenDisplayModalRef = ref<InstanceType<typeof Modal>>(); |
Other places in the codebase do this too. It's DEFINITELY just a nit.
