-
Notifications
You must be signed in to change notification settings - Fork 56
Dynamically created tabs are shown on explain and profile queries #23
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
f00bc64
to
528da71
Compare
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.
Check my comments
@@ -119,6 +146,38 @@ private void createUIComponents() { | |||
graphCanvas = new JPanel(new GridLayout(0, 1)); | |||
consoleTabsPane = new JBTabsPaneImpl(null, SwingConstants.TOP, this); | |||
consoleTabs = (JBTabsImpl) consoleTabsPane.getTabs(); | |||
|
|||
consoleTabs.addTabMouseListener(new MouseListener() { |
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.
There should be MouseAdaptor somewhere, that can be used to override only necessary methods.
final TabInfo info = consoleTabs.findInfo(e); | ||
if (info != null) { | ||
String tabTitle = info.getText(); | ||
if (tabTitle.startsWith("Profile") || tabTitle.startsWith("Explain")) { |
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.
If we are making comparisons using strings, then we should put them in constants and reuse constants
}); | ||
tabInfo.setTabLabelActions(tabActions, ActionPlaces.EDITOR_TAB); | ||
|
||
String planType = result.hasProfile() ? "Profile" : "Explain"; |
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.
Constants!
|
||
boolean hasProfile(); | ||
|
||
GraphQueryPlan getPlan(); |
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.
As I understand "hasPlan()" is main method. And it is accompanied with "getPlan()"
In this case "hasProfile" is misleading. I would prefer it be called "isProfilePlan()" to clearly define that it actually specifies type of plan.
.orElse(false); | ||
} | ||
|
||
public GraphQueryPlan getQueryPlan() { |
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 think we want to use Optional here.
f2f3075
to
8359c4a
Compare
No description provided.