Conversation
ryanclark
force-pushed
the
ryan/new-assist-ui
branch
from
ba5c0e7 to
142271f
Compare
Contributor
|
Some general UX observations: Empty output / failed executionsLooks like some of the improvements in #27010 got reverted. Loading an existing conversation where execution failed with an exit code just shows empty output: 
The 
"Connecting to nodes..." thoughtThe ellipsis sort of signals that the process is still ongoing, but the checkmark indicates that the process succeeded. 
PermalinksWe do not utilize history to be able to navigate / link to individual conversations anymore. Not sure that it would make sense to do so in a docked mode, but perhaps in the expanded mode, we could still "navigate" to conversation URLs? |
justinas
reviewed
Jun 9, 2023
ryanclark
force-pushed
the
ryan/new-assist-ui
branch
from
867faeb to
b10beef
Compare
justinas
approved these changes
Jun 9, 2023
kimlisa
reviewed
Jun 12, 2023
Contributor
kimlisa
left a comment
kimlisa
left a comment
There was a problem hiding this comment.
nothing too major stood out to me, overall great stuff
i did not test with a working assist or with MFA, i just tested whatever i could access without setting up open ai token.
if you want some help with testing with working assist, let me know (or do you have a cluster maybe?)
ryanclark
force-pushed
the
ryan/new-assist-ui
branch
from
c649be6 to
7c050b6
Compare
|
@ryanclark - this PR will require admin approval to merge due to its size. Consider breaking it up into a series smaller changes. |
kimlisa
approved these changes
Jun 12, 2023
Contributor
kimlisa
left a comment
kimlisa
left a comment
There was a problem hiding this comment.
LGTM, one other note is, i can click on the links now, but it unexpectedly closes the assist
Member
Author
Cheers, fixed |
ryanclark
force-pushed
the
ryan/new-assist-ui
branch
from
080e980 to
522541f
Compare
zmb3
approved these changes
Jun 12, 2023
…mandEntry.tsx Co-authored-by: Lisa Kim <lisa@goteleport.com>
ryanclark
force-pushed
the
ryan/new-assist-ui
branch
from
1663c4c to
b949dd5
Compare
|
@ryanclark See the table below for backport results.
|
ryanclark
added a commit
that referenced
this pull request
Jun 13, 2023
* Rewrite Assist & implement a new UI * Use the node name from the API * Code review comments * Address UI code review comments * Add some tests for the service * Rename "MiniAssit" to "Assist" * Impose a max height for the floating assist for small windows * Add missing license header * Update web/packages/teleport/src/Assist/Conversation/ExecuteRemoteCommandEntry.tsx Co-authored-by: Lisa Kim <lisa@goteleport.com> * Code review improvements * Only use stopPropagation so links still work and assist does not close * Make errors only show when error message isn't null * Add websocket refresh to avoid session expirations * Mark the conversation as not streaming when assist returns a full message * Improvements to error handling and icon spacing * Use encodeChallengeResponse to match the MFA backend changes * Run prettier * Usability improvements --------- Co-authored-by: Lisa Kim <lisa@goteleport.com>
ryanclark
added a commit
that referenced
this pull request
Jun 15, 2023
* [Assist] New UI & rewrite (#27629) * Rewrite Assist & implement a new UI * Use the node name from the API * Code review comments * Address UI code review comments * Add some tests for the service * Rename "MiniAssit" to "Assist" * Impose a max height for the floating assist for small windows * Add missing license header * Update web/packages/teleport/src/Assist/Conversation/ExecuteRemoteCommandEntry.tsx Co-authored-by: Lisa Kim <lisa@goteleport.com> * Code review improvements * Only use stopPropagation so links still work and assist does not close * Make errors only show when error message isn't null * Add websocket refresh to avoid session expirations * Mark the conversation as not streaming when assist returns a full message * Improvements to error handling and icon spacing * Use encodeChallengeResponse to match the MFA backend changes * Run prettier * Usability improvements --------- Co-authored-by: Lisa Kim <lisa@goteleport.com> * Update the proto encoding for MFA --------- Co-authored-by: Lisa Kim <lisa@goteleport.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This rewrites the data logic behind assist to be easier to test and clearer as to what is doing what.
The state is moved into a context which uses a reducer for all state operations. This means we have pure functions for manipulating state that can be tested.
New UI:
Collapsed -
Expanded -
Docked to sidebar -
TODO:
Resolve node ids to the hostname when displaying a conversation that previously had commands ran in it