Cody: Remove system notification from non-actions #51714
Merged
Conversation
This file contains 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
| @@ -7,7 +7,7 @@ export async function getAccessToken(secretStorage: SecretStorage): Promise<stri | |||
| return (await secretStorage.get(CODY_ACCESS_TOKEN_SECRET)) || null | |||
| } catch (error) { | |||
| await secretStorage.delete(CODY_ACCESS_TOKEN_SECRET) | |||
| void vscode.window.showErrorMessage(`Failed to retreive access token for Cody: ${error}`) | |||
| console.error(`Failed to retreive access token for Cody: ${error}`) | |||
There was a problem hiding this comment.
This seems unrelated to the issue. Was this an accidental commit?
There was a problem hiding this comment.
this was intentional because the UI should handle the errors, and we allow users to continue with initiating the webview anyway so I think it would make more sense to display this error in UI instead of popup. wdyt?
There was a problem hiding this comment.
Yeah I agree! Makes sense
Bundle size report 📦
Look at the Statoscope report for a full comparison between the commits 730b602 and 4827a3a or learn more. Open explanation
|
philipp-spiess
approved these changes
May 10, 2023
|
Perfect! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Close #51349
RE: https://sourcegraph.slack.com/archives/C04MZPE4JKD/p1683724897212539?thread_ts=1683723327.455849&cid=C04MZPE4JKD
PR to remove system notifications from non-actionable items, eg error messages as system pop up from background process etc
System notification should be handled by actionable functions individually.
VS code UX guideline: https://code.visualstudio.com/api/ux-guidelines/notifications
Test plan
Local dev.