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

Ide feature #7108

Merged
merged 5 commits into from
Dec 6, 2023
Merged

Ide feature #7108

merged 5 commits into from
Dec 6, 2023

Conversation

helin24
Copy link
Member

@helin24 helin24 commented Dec 4, 2023

Addresses #7100

CC @kenzieschmoll

*/
package io.flutter.devtools;

public enum DevToolsIdeFeature {
Copy link
Member

Choose a reason for hiding this comment

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

LGTM.

Add a URL reference here or in DevToolsUrl.java that points to #7100 for documentation.

@@ -275,7 +277,8 @@ private void addBrowserInspectorViewContent(FlutterApp app,
color,
UIUtil.getFontSize(UIUtil.FontSize.NORMAL),
flutterSdkVersion,
WorkspaceCache.getInstance(app.getProject())
WorkspaceCache.getInstance(app.getProject()),
openOnAppLaunch ? DevToolsIdeFeature.ON_DEBUG_AUTOMATIC : DevToolsIdeFeature.TOOL_WINDOW
Copy link
Member

@kenzieschmoll kenzieschmoll Dec 5, 2023

Choose a reason for hiding this comment

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

is it possible that a user can have the openOnAppLaunch setting on, and also manually open the Inspector tool window? I would think we would only want ON_DEBUG_AUTOMATIC if a user did not manually open the tool window.

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm yeah a user could open their project, and then before they run their app open the inspector tool window. I can change how we're setting openOnAppLaunch to only be true if the tool window was automatically opened.

Copy link
Member

@kenzieschmoll kenzieschmoll left a comment

Choose a reason for hiding this comment

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

Open question: can we differentiate when the user manually opened the tool window vs when the tool window was already opened (not automatically opened due to a setting) and therefore automatically showed the inspector when a debug session was started?

@helin24
Copy link
Member Author

helin24 commented Dec 5, 2023

Open question: can we differentiate when the user manually opened the tool window vs when the tool window was already opened (not automatically opened due to a setting) and therefore automatically showed the inspector when a debug session was started?

The last commit distinguishes these two, but to be clear, let's make sure these examples would be labeled as expected:

  1. A user starts with tool window closed but with setting "auto open on app run" true. When they run an app, the tool window opens without their input and DevTools appears --> ON_DEBUG_AUTOMATIC
  2. The same user stops running their app, but the tool window remains open. They run their app again, and DevTools appears --> TOOL_WINDOW
  3. The same user stops running their app and closes the tool window. They run their app again, and the tool window opens automatically, DevTools appears --> ON_DEBUG_AUTOMATIC

Is that what we're hoping for? Case 2 seems a little odd to me because the tool window doesn't close when an app stops running (should it? probably a bigger change though)

@kenzieschmoll
Copy link
Member

kenzieschmoll commented Dec 5, 2023

Case 2 seems fine to me because this scenario could happen for a user regardless of whether they have the on_debug_automatic setting enabled or not.

This makes me think though, should we further distinguish TOOL_WINDOW to be TOOL_WINDOW_RELOAD and TOOL_WINDOW_OPEN ? Which would distinguish when the tool window loads devtools because it was already open vs when the tool window loads devtools because the user clicked on the tool window tab.

@jacob314 thoughts?

@jacob314
Copy link
Contributor

jacob314 commented Dec 6, 2023

Case 2 seems fine to me because this scenario could happen for a user regardless of whether they have the on_debug_automatic setting enabled or not.

This makes me think though, should we further distinguish TOOL_WINDOW to be TOOL_WINDOW_RELOAD and TOOL_WINDOW_OPEN ? Which would distinguish when the tool window loads devtools because it was already open vs when the tool window loads devtools because the user clicked on the tool window tab.

@jacob314 thoughts?

Seems reasonable if it makes it easier to get apples-to-apples comparisons between VSCode and IntelliJ usage. Aside from that, I'm not sure it matters much as daily/weekly active user and session length metrics are probably more important than raw numbers of opens for understanding adoption.

@helin24
Copy link
Member Author

helin24 commented Dec 6, 2023

I think it would be possible to distinguish TOOL_WINDOW_RELOAD and TOOL_WINDOW_OPEN, but I'm not sure if this is useful for comparing with VS Code - @kenzieschmoll I'll leave it up to you.

@helin24
Copy link
Member Author

helin24 commented Dec 6, 2023

This will now distinguish the reload and open cases

@helin24 helin24 merged commit 0249a03 into flutter:master Dec 6, 2023
7 checks passed
@helin24 helin24 deleted the ide-feature branch December 6, 2023 19:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants