-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Add a share button when logged into cloud #4448
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
jr
left a comment
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.
Seems good generally. Left some comments about stuff I'd consider cleaning up a little.
Ideally we'd also handle the case where your organization has not enabled sharing more gracefully: right now it leads to a "failed, try again" message that won't actually be able to succeed unless the org admins change their settings.
| return false | ||
| } | ||
|
|
||
| const response = await axios.post( |
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.
We might consider using fetch instead of axios. I have an incomplete cleanup task to remove axios from AuthService that I keep not getting to a clean state.
Main motivation (other than not using a library unless we have to): as far as I can tell you can do a better job of setting timeouts with fetch than axios, which is something we should start doing for most of these requests (another cleanup task I haven't gotten to yet). Hopefully those will be better examples for Roo too, although maybe that's too optimistic.
That would mean you'd need to explicitly check the status & fail on non-2xx, since that behavior's different between fetch and axios.
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 like the idea of changing all of the axioses to fetches as a follow up and adding a rules file with instructions to use fetch instead going forward.
Important
Adds task sharing functionality with a new
ShareService, UI updates, and i18n support for multiple languages.ShareServiceclass inShareService.tsto handle task sharing, including creating share links and copying them to the clipboard.ShareServiceintoCloudServiceinCloudService.ts, addingshareTask()andcanShareTask()methods.webviewMessageHandler.tsto handleshareCurrentTaskmessage, showing success or error messages.TaskActions.tsxinwebview-ui/src/components/chat/whensharingEnabledis true.ClineProvider.tsto includesharingEnabledstate.ShareService.test.tsto testShareServicefunctionality.getUserAgent()function inutils.tsfor consistent user-agent string.webview-ui/src/i18n/locales/to include translations for share task functionality.This description was created by
for 4e5122c. You can customize this summary. It will automatically update as commits are pushed.