Skip to content
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

chore: add tracker to editor's paste and OTel config load through URL #138

Merged
merged 6 commits into from
Oct 20, 2023

Conversation

roshan-gh
Copy link
Contributor

No description provided.

@roshan-gh roshan-gh self-assigned this Oct 19, 2023
@roshan-gh roshan-gh linked an issue Oct 19, 2023 that may be closed by this pull request
2 tasks
@vercel
Copy link

vercel bot commented Oct 19, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
otelbin 🔄 Building (Inspect) Visit Preview 💬 Add feedback Oct 20, 2023 1:02pm

@@ -77,6 +78,9 @@ export default function Editor({ locked, setLocked }: { locked: boolean; setLock
// /restore page. Without this we ran into return URL problems with GitHub
// and Google as our return URL can be **very** long.
localStorage.setItem("config-restore", config);
if (config !== editorBinding.fallback) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Issue: We must only report this event once on the initial page load

Copy link
Contributor Author

@roshan-gh roshan-gh Oct 20, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Initial page load even with default config? I assumed load the page with different config that stored in URL. @bripkens

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assumed load the page with different config that stored in URL.

Yes, correct. It should be both conditions: Initial load and when the config is different from the default one.

@@ -240,6 +241,10 @@ export const EditorProvider = ({ children }: { children: ReactNode }) => {
};
findSymbols(docObject, "", wordAtCursor.word, cursorOffset);
});

editorRef.current.onDidPaste(() => {
track("Paste", { location: "Editor" });
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have just been thinking about this event. This is likely too fine-grained tracking and it is hard to interpret. Let's delete this event (sorry for the back and forth).

@@ -240,6 +241,10 @@ export const EditorProvider = ({ children }: { children: ReactNode }) => {
};
findSymbols(docObject, "", wordAtCursor.word, cursorOffset);
});

editorRef.current.onDidPaste(() => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great to see how this works. We can make use of this handler for function for this task :)

#99

@bripkens bripkens merged commit 6bf797c into main Oct 20, 2023
3 of 4 checks passed
@bripkens bripkens deleted the 136-track-feature-usage-of-otelbin branch October 20, 2023 13:02
@sonarcloud
Copy link

sonarcloud bot commented Oct 20, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@github-actions github-actions bot locked and limited conversation to collaborators Oct 20, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Track feature usage of OTelBin
2 participants