-
Notifications
You must be signed in to change notification settings - Fork 12
Pull request ai summary button #2009
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
Conversation
…ead of its own textarea
…es screen when creating a PR
✅ Deploy Preview for harness-xd-review ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for harness-design ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
| const { fullGitRef: baseRef } = useGitRef() | ||
|
|
||
| const mutation = useMutation(async ({ repoMetadata, baseRef, headRef }: AiPullRequestSummaryParams) => { | ||
| return fetch(getApiPath(`/api/v1/repos/${repoMetadata.path}/+/genai/change-summary`), { |
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.
Is this API not available on the Code service React query client?
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.
Correct. @abhinavrastogi-harness has called this out as a gap to the backend team.
| }} | ||
| /> | ||
|
|
||
| {showAiLoader && ( |
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.
NIT: Instead of blocking the textarea, should we display a loader within the button to indicate loading state?
Thoughts?
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 did try using the loading icon within the button as we discussed in our standup, but the behavior didn't really look right to me.
The loading spinner within the button appeared to the left of the AI icon. Since this is just an icon button, the width of the button remained the same and the two icons overflowed and were clipped. We could fix that and widen the button when the loader appears, but not sure that would be right either since it means all of the other buttons would then jog to the right while the loader appears.
Open to that behavior though if others feel that's what we want.
As for the blocking, I think we concluded we would block the editor for first cut to reduce corner cases like typing and changing the cursor selection index before the injected text returns from the async call. We can certainly workaround all those issues but figured we'd circle back after some feedback on whether it was worth the effort or not.
.../ui/src/views/repo/pull-request/details/components/conversation/pull-request-comment-box.tsx
Outdated
Show resolved
Hide resolved
| const isFirst = index === 0 | ||
| return ( | ||
| <Fragment key={`${comment}-${index}`}> | ||
| <Button |
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.
Should we need to disable the button when AI call is in progress?
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.
Oooo. Good call out. Let me actually make this change before merging. Thanks!
…versation/pull-request-comment-box.tsx Co-authored-by: praneshg239 <95267551+praneshg239@users.noreply.github.com>
No description provided.