Add survey and banner #7
Conversation
src/client/banner.ts
Outdated
| openCommand = '/usr/bin/xdg-open'; | ||
| } | ||
| if (!openCommand) { | ||
| console.error(`Unable to determine platform to capture user feedback in Python extension ${os.platform()}`); |
There was a problem hiding this comment.
Is this for feedback or a generic banner system? And should the error message include the URL in case people want to manually navigate to it?
There was a problem hiding this comment.
This is the generic banner.
Agreed, will include URL
There was a problem hiding this comment.
OK, then maybe we should change it from "Unable to determine platform to capture user feedback in Python extension" to "Unable to determine platform in order to open ${BANNER_URL}" or something.
src/client/common/persistentState.ts
Outdated
| } | ||
| } | ||
|
|
||
| export interface IPersistentStateFactor { |
src/client/feedback/counters.ts
Outdated
| super(); | ||
| this.createCounters(); | ||
| } | ||
| public updateEditCounter(): void { |
There was a problem hiding this comment.
incrementEditCounter since update suggests you can do more than just an increment. (same with the other update methods.)
src/client/feedback/counters.ts
Outdated
|
|
||
| this.checkThreshold(); | ||
| } | ||
| private checkThreshold() { |
There was a problem hiding this comment.
Should this take counterName so that just the single value is checked? Do you expect the counters to be updated outside of this instance to lead to needing to check all counters?
There was a problem hiding this comment.
Aarg, carry on from old code where I was persisting counters between sessions and this check was run only every minute or so.
| if (telemetryEventName === FEEDBACK) { | ||
| return; | ||
| } | ||
| if (!this.showFeedbackPrompt.value || this.userResponded.value || !this.counters) { |
There was a problem hiding this comment.
This if is used twice. Worth factoring out into a method?
| private displaySurvey() { | ||
| this.userResponded.value = true; | ||
|
|
||
| let openCommand: string | undefined; |
There was a problem hiding this comment.
Is this code common between here and the banner? Should it be factored out?
There was a problem hiding this comment.
Yes, but the banner would go out (removed), hence didn't want to share any code.
|
|
||
| 'use strict'; | ||
|
|
||
| export * from './feedbackService'; |
There was a problem hiding this comment.
What's this file for? Simplifying the namespace?
src/client/jupyter/main.ts
Outdated
| @@ -7,7 +7,7 @@ import { Commands, PythonLanguage } from '../common/constants'; | |||
| import { JupyterCodeLensProvider } from './editorIntegration/codeLensProvider'; | |||
| import { JupyterSymbolProvider } from './editorIntegration/symbolProvider'; | |||
| import { formatErrorForLogging } from '../common/utils'; | |||
| // import * as telemetryHelper from '../common/telemetry'; | |||
| // import * as telemetryHelper from '../telemetry'; | |||
There was a problem hiding this comment.
Any reason to keep around the commented-out imports?
…-python into SurveyAndBanner * 'SurveyAndBanner' of https://github.com/Microsoft/vscode-python: check whether to display banner
|
@brettcannon , fixed all, please re-review while tests run. |
Implement basic webview support. This should allow us to iterate on the datascience controls using react. This commit creates a history pane that just looks like the default "react" page but works inside of a webview panel. Additionally it still makes it possible to load the index.html into a browser for debugging the react code standalone.
Implement basic webview support. This should allow us to iterate on the datascience controls using react. This commit creates a history pane that just looks like the default "react" page but works inside of a webview panel. Additionally it still makes it possible to load the index.html into a browser for debugging the react code standalone.
No description provided.