-
Notifications
You must be signed in to change notification settings - Fork 4
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
Trigger customization notifications on ide, stacking multiple notifications vertically and refactor/add support for more methods in PluginStore #48
Changes from all commits
ec98e04
5e49228
3ba6cc7
6de13f6
617e53e
5f2b1d1
37bc88f
4d793b7
013f639
d3b81da
da58a94
2cc45d4
5fc9c59
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,12 +7,15 @@ | |
import java.util.HashMap; | ||
import java.util.List; | ||
import java.util.Map; | ||
import java.util.Objects; | ||
import java.util.concurrent.CompletableFuture; | ||
import java.util.concurrent.ExecutionException; | ||
|
||
import org.eclipse.jface.dialogs.Dialog; | ||
import org.eclipse.jface.dialogs.IDialogConstants; | ||
import org.eclipse.swt.SWT; | ||
import org.eclipse.swt.events.MouseAdapter; | ||
import org.eclipse.swt.events.MouseEvent; | ||
import org.eclipse.swt.events.SelectionAdapter; | ||
import org.eclipse.swt.events.SelectionEvent; | ||
import org.eclipse.swt.graphics.Font; | ||
|
@@ -34,7 +37,9 @@ | |
import software.aws.toolkits.eclipse.amazonq.util.Constants; | ||
import software.aws.toolkits.eclipse.amazonq.util.PluginLogger; | ||
import software.aws.toolkits.eclipse.amazonq.util.ThreadingUtils; | ||
import software.aws.toolkits.eclipse.amazonq.util.ToolkitNotification; | ||
import software.aws.toolkits.eclipse.amazonq.views.model.Customization; | ||
import org.eclipse.mylyn.commons.ui.dialogs.AbstractNotificationPopup; | ||
|
||
public final class CustomizationDialog extends Dialog { | ||
|
||
|
@@ -45,7 +50,7 @@ public final class CustomizationDialog extends Dialog { | |
private Font boldFont; | ||
private List<Customization> customizationsResponse; | ||
private ResponseSelection responseSelection; | ||
private String selectedCustomisationArn; | ||
private Customization selectedCustomization; | ||
|
||
public enum ResponseSelection { | ||
AMAZON_Q_FOUNDATION_DEFAULT, | ||
|
@@ -80,6 +85,10 @@ private final class CustomRadioButton extends Composite { | |
public Button getRadioButton() { | ||
return radioButton; | ||
} | ||
|
||
public Label getTextLabel() { | ||
return textLabel; | ||
} | ||
} | ||
|
||
public CustomizationDialog(final Shell parentShell) { | ||
|
@@ -102,12 +111,19 @@ public ResponseSelection getResponseSelection() { | |
return this.responseSelection; | ||
} | ||
|
||
public void setSelectedCustomizationArn(final String arn) { | ||
this.selectedCustomisationArn = arn; | ||
public void setSelectedCustomization(final Customization customization) { | ||
this.selectedCustomization = customization; | ||
} | ||
|
||
public Customization getSelectedCustomization() { | ||
return this.selectedCustomization; | ||
} | ||
|
||
public String getSelectedCustomizationArn() { | ||
return this.selectedCustomisationArn; | ||
private void showNotification(final String customizationName) { | ||
AbstractNotificationPopup notification = new ToolkitNotification(Display.getCurrent(), | ||
Constants.IDE_CUSTOMIZATION_NOTIFICATION_TITLE, | ||
String.format(Constants.IDE_CUSTOMIZATION_NOTIFICATION_BODY_TEMPLATE, customizationName)); | ||
notification.open(); | ||
} | ||
|
||
@Override | ||
|
@@ -118,16 +134,17 @@ protected void createButtonsForButtonBar(final Composite parent) { | |
|
||
@Override | ||
protected void okPressed() { | ||
PluginLogger.info(String.format("Select pressed with responseSelection:%s and selectedArn:%s", this.responseSelection, this.selectedCustomisationArn)); | ||
if (this.responseSelection.equals(ResponseSelection.AMAZON_Q_FOUNDATION_DEFAULT)) { | ||
PluginStore.remove(Constants.CUSTOMIZATION_STORAGE_INTERNAL_KEY); | ||
} else if (this.selectedCustomisationArn != null) { | ||
PluginStore.put(Constants.CUSTOMIZATION_STORAGE_INTERNAL_KEY, this.selectedCustomisationArn); | ||
Display.getCurrent().asyncExec(() -> showNotification(Constants.DEFAULT_Q_FOUNDATION_DISPLAY_NAME)); | ||
} else if (Objects.nonNull(this.getSelectedCustomization()) && StringUtils.isNotBlank(this.getSelectedCustomization().getName())) { | ||
PluginStore.putObject(Constants.CUSTOMIZATION_STORAGE_INTERNAL_KEY, this.getSelectedCustomization()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we not just store the ARN? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Earlier I was just storing ARN only but I need to have name (or maybe description as well later) for showing notifications. So instead of creating 3 different getter/setter for all of these, I changed the implementation to just have a single setter/getter. |
||
Map<String, Object> updatedSettings = new HashMap<>(); | ||
Map<String, String> internalMap = new HashMap<>(); | ||
internalMap.put(Constants.LSP_CUSTOMIZATION_CONFIGURATION_KEY, this.selectedCustomisationArn); | ||
internalMap.put(Constants.LSP_CUSTOMIZATION_CONFIGURATION_KEY, this.getSelectedCustomization().getArn()); | ||
updatedSettings.put(Constants.LSP_CONFIGURATION_KEY, internalMap); | ||
ThreadingUtils.executeAsyncTask(() -> CustomizationUtil.triggerChangeConfigurationNotification(updatedSettings)); | ||
Display.getCurrent().asyncExec(() -> showNotification(String.format("%s customization", this.getSelectedCustomization().getName()))); | ||
} | ||
super.okPressed(); | ||
} | ||
|
@@ -179,10 +196,10 @@ private void updateComboOnUIThread(final List<Customization> customizations) { | |
int defaultSelectedDropdownIndex = -1; | ||
for (int index = 0; index < customizations.size(); index++) { | ||
addFormattedOption(combo, customizations.get(index).getName(), customizations.get(index).getDescription()); | ||
combo.setData(String.format("%s", index), customizations.get(index).getArn()); | ||
combo.setData(String.format("%s", index), customizations.get(index)); | ||
if (this.responseSelection.equals(ResponseSelection.CUSTOMIZATION) | ||
&& StringUtils.isNotBlank(this.selectedCustomisationArn) | ||
&& this.selectedCustomisationArn.equals(customizations.get(index).getArn())) { | ||
&& Objects.nonNull(this.getSelectedCustomization()) | ||
&& this.getSelectedCustomization().getArn().equals(customizations.get(index).getArn())) { | ||
defaultSelectedDropdownIndex = index; | ||
} | ||
} | ||
|
@@ -197,9 +214,9 @@ private void updateComboOnUIThread(final List<Customization> customizations) { | |
public void widgetSelected(final SelectionEvent e) { | ||
int selectedIndex = combo.getSelectionIndex(); | ||
String selectedOption = combo.getItem(selectedIndex); | ||
String selectedCustomizationArn = (String) combo.getData(String.valueOf(selectedIndex)); | ||
CustomizationDialog.this.selectedCustomisationArn = selectedCustomizationArn; | ||
PluginLogger.info(String.format("Selected option:%s with arn:%s", selectedOption, selectedCustomizationArn)); | ||
Customization selectedCustomization = (Customization) combo.getData(String.valueOf(selectedIndex)); | ||
CustomizationDialog.this.setSelectedCustomization(selectedCustomization); | ||
PluginLogger.info(String.format("Selected option:%s with arn:%s", selectedOption, selectedCustomization.getArn())); | ||
} | ||
}); | ||
} | ||
|
@@ -254,7 +271,17 @@ protected Control createDialogArea(final Composite parent) { | |
public void widgetSelected(final SelectionEvent e) { | ||
customizationButton.getRadioButton().setSelection(false); | ||
responseSelection = ResponseSelection.AMAZON_Q_FOUNDATION_DEFAULT; | ||
selectedCustomisationArn = null; | ||
setSelectedCustomization(null); | ||
combo.setEnabled(false); | ||
} | ||
}); | ||
defaultAmazonQFoundationButton.getTextLabel().addMouseListener(new MouseAdapter() { | ||
@Override | ||
public void mouseDown(final MouseEvent e) { | ||
customizationButton.getRadioButton().setSelection(false); | ||
defaultAmazonQFoundationButton.getRadioButton().setSelection(true); | ||
responseSelection = ResponseSelection.AMAZON_Q_FOUNDATION_DEFAULT; | ||
setSelectedCustomization(null); | ||
combo.setEnabled(false); | ||
} | ||
}); | ||
|
@@ -266,6 +293,15 @@ public void widgetSelected(final SelectionEvent e) { | |
combo.setEnabled(true); | ||
} | ||
}); | ||
customizationButton.getTextLabel().addMouseListener(new MouseAdapter() { | ||
@Override | ||
public void mouseDown(final MouseEvent e) { | ||
defaultAmazonQFoundationButton.getRadioButton().setSelection(false); | ||
customizationButton.getRadioButton().setSelection(true); | ||
responseSelection = ResponseSelection.CUSTOMIZATION; | ||
combo.setEnabled(true); | ||
} | ||
}); | ||
createDropdownForCustomizations(container); | ||
createSeparator(container); | ||
return container; | ||
|
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.
While this technically can be done serializing entire objects does present additional risks. In general I would like to be convinced that use cases require it, vs. binding relational configuration under a common key namespace, e.g.:
notifications.foo
:123
notifications.bar
:456
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.
For the same reason which I described here https://github.com/aws/amazon-q-eclipse/pull/48/files#r1787324480
Also, currently when LSP server requests for aws.q, we only return customization object but maybe in near future - we might be returning more than just one single row item. Then in those kindof cases, it would be better to just do a single lookup and return it.