Conversation
This PR port all WebUI changes used by Teleport Assist.
|
@jakule - this PR will require admin approval to merge due to its size. Consider breaking it up into a series smaller changes. |
Remove console.log
| break; | ||
|
|
||
| case MessageTypeEnum.RAW: | ||
| const data = JSON.parse(msg.payload) as RawPayload; |
There was a problem hiding this comment.
Should we be catching SyntaxErrors here? I noticed you did so in the useLocalStorage hook, but not here.
There was a problem hiding this comment.
We could maybe start to make use of zod in the web UI in the near future. We use it in Connect to validate and cast to specific types, could be useful here (not a blocker for this now, just rambling)
avatus
left a comment
avatus
left a comment
There was a problem hiding this comment.
In general, we've defined all the styled component stuff at the bottom of the file instead of the top. It might be a bit of a large task to migrate them all to the bottom but consistency would be good. Maybe a PR in the future?
I wasn't able to test locally without an API key so this was a glance at this PR (and the original PRs to dev-ai) and seems good enough, but admittedly, the size got me lost in the sauce.
| @@ -0,0 +1,46 @@ | |||
| /* | |||
There was a problem hiding this comment.
is this avatar distinct to chat only or can it be moved to its own component in design
| break; | ||
|
|
||
| case MessageTypeEnum.RAW: | ||
| const data = JSON.parse(msg.payload) as RawPayload; |
There was a problem hiding this comment.
We could maybe start to make use of zod in the web UI in the near future. We use it in Connect to validate and cast to specific types, could be useful here (not a blocker for this now, just rambling)
There was a problem hiding this comment.
most of my comments are not blockers (they can be dealt with later), the only thing i'll express concern though is inconsistent error handling. there are a few places where we don't check for error (api calls, awaits) so i'm guessing outcome is either breakage or nothing happens for the user
There was a problem hiding this comment.
can we defined this in our SVGIcon/*? i created this directory (assets/images/icons/*) not realizing we had the svg directory. unless there was a specific reason we should put them here? (i created a svg container for the integration list recently here)
There was a problem hiding this comment.
@justinas deferring to you as this is icon is for the integrations page, which uses svg files instead of SVGIcon
There was a problem hiding this comment.
IMO it's fine as-is for this PR, I can try and move all the integration icons to SVGIcon as a follow up.
Co-authored-by: Lisa Kim <lisa@goteleport.com>
kimlisa
left a comment
kimlisa
left a comment
There was a problem hiding this comment.
thanks for changes, just some other small stuff and looks good
public-teleport-github-review-bot
Bot
removed the request for review
from ravicious
7 participants
This PR ports all WebUI changes used by Teleport Assist.
Note: The backend changes are not included in this branch, so UI won't show up if run directly on this branch.