On 17 September 2012 21:12, Philippe Mouawad <philippe.moua...@gmail.com> wrote: > Hello sebb, > Yes they are. > I changed code to ensure we don't use them needlessly, so mispelling cannot > occur anymore with this code > If you think they should be externalized, then isn't ActionNames the best > place ?
Are the part of the same namespace as ActionNames? i.e. do they have to be different from any existing entries in ActionNames? If they are part of a different namespace then I think they should probably be defined in a separate class, as it might be confusing to have two namespaces defined in the same class. And it would be harder to write a test case that checked for string value uniqueness if the namespaces were in the same class. > Regards > Philippe. > > On Mon, Sep 17, 2012 at 6:12 PM, sebb <seb...@gmail.com> wrote: > >> On 16 September 2012 11:06, <pmoua...@apache.org> wrote: >> > Author: pmouawad >> > Date: Sun Sep 16 10:06:30 2012 >> > New Revision: 1385239 >> > >> > URL: http://svn.apache.org/viewvc?rev=1385239&view=rev >> > Log: >> > Bug 53879 - GUI : Allow Popups to be closed with ESC key >> > Bugzilla Id: 53879 >> > >> > Added: >> > jmeter/trunk/src/core/org/apache/jmeter/gui/util/EscapeDialog.java >> (with props) >> > Modified: >> > >> jmeter/trunk/src/core/org/apache/jmeter/config/gui/RowDetailDialog.java >> > >> jmeter/trunk/src/core/org/apache/jmeter/functions/gui/FunctionHelper.java >> > jmeter/trunk/src/core/org/apache/jmeter/gui/MainFrame.java >> > jmeter/trunk/src/core/org/apache/jmeter/gui/action/AboutCommand.java >> > jmeter/trunk/src/core/org/apache/jmeter/gui/action/Help.java >> > >> jmeter/trunk/src/core/org/apache/jmeter/gui/action/SearchTreeDialog.java >> > jmeter/trunk/xdocs/changes.xml >> > >> > Modified: >> jmeter/trunk/src/core/org/apache/jmeter/config/gui/RowDetailDialog.java >> > URL: >> http://svn.apache.org/viewvc/jmeter/trunk/src/core/org/apache/jmeter/config/gui/RowDetailDialog.java?rev=1385239&r1=1385238&r2=1385239&view=diff >> > >> ============================================================================== >> > --- >> jmeter/trunk/src/core/org/apache/jmeter/config/gui/RowDetailDialog.java >> (original) >> > +++ >> jmeter/trunk/src/core/org/apache/jmeter/config/gui/RowDetailDialog.java Sun >> Sep 16 10:06:30 2012 >> > @@ -24,13 +24,18 @@ import java.awt.FlowLayout; >> > import java.awt.event.ActionEvent; >> > import java.awt.event.ActionListener; >> > >> > +import javax.swing.AbstractAction; >> > +import javax.swing.Action; >> > +import javax.swing.ActionMap; >> > import javax.swing.BorderFactory; >> > import javax.swing.BoxLayout; >> > +import javax.swing.InputMap; >> > import javax.swing.JButton; >> > import javax.swing.JComponent; >> > import javax.swing.JDialog; >> > import javax.swing.JFrame; >> > import javax.swing.JPanel; >> > +import javax.swing.JRootPane; >> > >> > import org.apache.jmeter.gui.action.KeyStrokes; >> > import org.apache.jmeter.util.JMeterUtils; >> > @@ -76,24 +81,6 @@ public class RowDetailDialog extends JDi >> > public RowDetailDialog() { >> > super(); >> > } >> > - /** >> > - * Hide Window on ESC >> > - */ >> > - private final transient ActionListener enterActionListener = new >> ActionListener() { >> > - public void actionPerformed(ActionEvent actionEvent) { >> > - doUpdate(actionEvent); >> > - setVisible(false); >> > - } >> > - }; >> > - >> > - /** >> > - * Do search on Enter >> > - */ >> > - private final transient ActionListener escapeActionListener = >> new ActionListener() { >> > - public void actionPerformed(ActionEvent actionEvent) { >> > - setVisible(false); >> > - } >> > - }; >> > >> > public RowDetailDialog(ObjectTableModel tableModel, int >> selectedRow) { >> > super((JFrame) null, JMeterUtils.getResString("detail"), true); >> //$NON-NLS-1$ >> > @@ -102,6 +89,31 @@ public class RowDetailDialog extends JDi >> > init(); >> > } >> > >> > + @Override >> > + protected JRootPane createRootPane() { >> > + JRootPane rootPane = new JRootPane(); >> > + InputMap inputMap = >> rootPane.getInputMap(JComponent.WHEN_IN_FOCUSED_WINDOW); >> > + ActionMap actionMap = rootPane.getActionMap(); >> > + inputMap.put(KeyStrokes.ESC, "ESCAPE"); >> > + inputMap.put(KeyStrokes.ENTER, "ENTER"); >> >> Are these strings defined anywhere, or are they arbitrary? >> >> Is there a requirement for the InputMap entry values to be unique? >> Do the inpuMap values have to be the same across classes? >> >> Seems to me it would be better to have a class that defines the strings. >> That would make it easier to spot accidental duplicates / >> misspellings, as well as documenting the values JMeter uses. >> >> i.e. something like the approach in ActionNames.java >> >> > + // Hide Window on ESC >> > + Action escapeAction = new AbstractAction() { >> > + public void actionPerformed(ActionEvent actionEvent) { >> > + setVisible(false); >> > + } >> > + }; >> > + actionMap.put("ESCAPE", escapeAction); >> > + // Do update on Enter >> > + Action enterAction = new AbstractAction() { >> > + public void actionPerformed(ActionEvent actionEvent) { >> > + doUpdate(actionEvent); >> > + setVisible(false); >> > + } >> > + }; >> > + actionMap.put("ENTER", enterAction); >> > + return rootPane; >> > + } >> > + >> > private void init() { >> > this.getContentPane().setLayout(new BorderLayout(10,10)); >> > >> > @@ -143,8 +155,6 @@ public class RowDetailDialog extends JDi >> > buttonsPanel.add(closeButton); >> > mainPanel.add(buttonsPanel, BorderLayout.SOUTH); >> > this.getContentPane().add(mainPanel); >> > - mainPanel.registerKeyboardAction(enterActionListener, >> KeyStrokes.ENTER, JComponent.WHEN_IN_FOCUSED_WINDOW); >> > - mainPanel.registerKeyboardAction(escapeActionListener, >> KeyStrokes.ESC, JComponent.WHEN_IN_FOCUSED_WINDOW); >> > nameTF.requestFocusInWindow(); >> > >> > this.pack(); >> > >> > Modified: >> jmeter/trunk/src/core/org/apache/jmeter/functions/gui/FunctionHelper.java >> > URL: >> http://svn.apache.org/viewvc/jmeter/trunk/src/core/org/apache/jmeter/functions/gui/FunctionHelper.java?rev=1385239&r1=1385238&r2=1385239&view=diff >> > >> ============================================================================== >> > --- >> jmeter/trunk/src/core/org/apache/jmeter/functions/gui/FunctionHelper.java >> (original) >> > +++ >> jmeter/trunk/src/core/org/apache/jmeter/functions/gui/FunctionHelper.java >> Sun Sep 16 10:06:30 2012 >> > @@ -25,10 +25,16 @@ import java.awt.event.ActionListener; >> > import java.util.Arrays; >> > import java.util.Comparator; >> > import java.util.List; >> > + >> > +import javax.swing.AbstractAction; >> > +import javax.swing.InputMap; >> > import javax.swing.JButton; >> > +import javax.swing.JComponent; >> > import javax.swing.JDialog; >> > import javax.swing.JFrame; >> > import javax.swing.JPanel; >> > +import javax.swing.JRootPane; >> > +import javax.swing.KeyStroke; >> > import javax.swing.event.ChangeEvent; >> > import javax.swing.event.ChangeListener; >> > >> > @@ -39,6 +45,7 @@ import org.apache.jmeter.engine.util.Com >> > import org.apache.jmeter.functions.Function; >> > import org.apache.jmeter.gui.action.ActionRouter; >> > import org.apache.jmeter.gui.action.Help; >> > +import org.apache.jmeter.gui.action.KeyStrokes; >> > import org.apache.jmeter.testelement.property.PropertyIterator; >> > import org.apache.jmeter.util.JMeterUtils; >> > import org.apache.jmeter.util.LocaleChangeEvent; >> > @@ -61,7 +68,30 @@ public class FunctionHelper extends JDia >> > init(); >> > JMeterUtils.addLocaleChangeListener(this); >> > } >> > - >> > + >> > + /** >> > + * Allow Dialog to be closed by ESC key >> > + */ >> > + @Override >> > + protected JRootPane createRootPane() { >> > + JRootPane rootPane = new JRootPane(); >> > + KeyStroke stroke = KeyStrokes.ESC; >> > + InputMap inputMap = >> rootPane.getInputMap(JComponent.WHEN_IN_FOCUSED_WINDOW); >> > + javax.swing.Action actionListener = new AbstractAction() { >> > + /** >> > + * >> > + */ >> > + private static final long serialVersionUID = >> -4036804004190858925L; >> > + >> > + public void actionPerformed(ActionEvent actionEvent) { >> > + setVisible(false); >> > + } >> > + }; >> > + inputMap.put(stroke, "ESCAPE"); >> > + rootPane.getActionMap().put("ESCAPE", actionListener); >> > + return rootPane; >> > + } >> > + >> > private void init() { >> > parameterPanel = new >> ArgumentsPanel(JMeterUtils.getResString("function_params"), false); >> //$NON-NLS-1$ >> > initializeFunctionList(); >> > @@ -80,6 +110,7 @@ public class FunctionHelper extends JDia >> > generateButton.addActionListener(this); >> > resultsPanel.add(generateButton); >> > this.getContentPane().add(resultsPanel, BorderLayout.SOUTH); >> > + >> > this.pack(); >> > ComponentUtil.centerComponentInWindow(this); >> > } >> > >> > Modified: jmeter/trunk/src/core/org/apache/jmeter/gui/MainFrame.java >> > URL: >> http://svn.apache.org/viewvc/jmeter/trunk/src/core/org/apache/jmeter/gui/MainFrame.java?rev=1385239&r1=1385238&r2=1385239&view=diff >> > >> ============================================================================== >> > --- jmeter/trunk/src/core/org/apache/jmeter/gui/MainFrame.java (original) >> > +++ jmeter/trunk/src/core/org/apache/jmeter/gui/MainFrame.java Sun Sep >> 16 10:06:30 2012 >> > @@ -74,6 +74,7 @@ import org.apache.jmeter.gui.action.Acti >> > import org.apache.jmeter.gui.action.LoadDraggedFile; >> > import org.apache.jmeter.gui.tree.JMeterCellRenderer; >> > import org.apache.jmeter.gui.tree.JMeterTreeListener; >> > +import org.apache.jmeter.gui.util.EscapeDialog; >> > import org.apache.jmeter.gui.util.JMeterMenuBar; >> > import org.apache.jmeter.gui.util.JMeterToolBar; >> > import org.apache.jmeter.samplers.Clearable; >> > @@ -341,7 +342,7 @@ public class MainFrame extends JFrame im >> > if (stoppingMessage != null){ >> > stoppingMessage.dispose(); >> > } >> > - stoppingMessage = new JDialog(this, >> JMeterUtils.getResString("stopping_test_title"), true); //$NON-NLS-1$ >> > + stoppingMessage = new EscapeDialog(this, >> JMeterUtils.getResString("stopping_test_title"), true); //$NON-NLS-1$ >> > JLabel stopLabel = new >> JLabel(JMeterUtils.getResString("stopping_test") + ": " + host); >> //$NON-NLS-1$$NON-NLS-2$ >> > stopLabel.setBorder(BorderFactory.createEmptyBorder(20, 20, 20, >> 20)); >> > stoppingMessage.getContentPane().add(stopLabel); >> > >> > Modified: >> jmeter/trunk/src/core/org/apache/jmeter/gui/action/AboutCommand.java >> > URL: >> http://svn.apache.org/viewvc/jmeter/trunk/src/core/org/apache/jmeter/gui/action/AboutCommand.java?rev=1385239&r1=1385238&r2=1385239&view=diff >> > >> ============================================================================== >> > --- jmeter/trunk/src/core/org/apache/jmeter/gui/action/AboutCommand.java >> (original) >> > +++ jmeter/trunk/src/core/org/apache/jmeter/gui/action/AboutCommand.java >> Sun Sep 16 10:06:30 2012 >> > @@ -39,6 +39,7 @@ import javax.swing.SwingConstants; >> > import javax.swing.border.EmptyBorder; >> > >> > import org.apache.jmeter.gui.GuiPackage; >> > +import org.apache.jmeter.gui.util.EscapeDialog; >> > import org.apache.jmeter.util.JMeterUtils; >> > >> > /** >> > @@ -83,7 +84,7 @@ public class AboutCommand implements Com >> > void about() { >> > JFrame mainFrame = GuiPackage.getInstance().getMainFrame(); >> > if (about == null) { >> > - about = new JDialog(mainFrame, "About Apache JMeter...", >> false); >> > + about = new EscapeDialog(mainFrame, "About Apache >> JMeter...", false); >> > about.addMouseListener(new MouseAdapter() { >> > @Override >> > public void mouseClicked(MouseEvent e) { >> > >> > Modified: jmeter/trunk/src/core/org/apache/jmeter/gui/action/Help.java >> > URL: >> http://svn.apache.org/viewvc/jmeter/trunk/src/core/org/apache/jmeter/gui/action/Help.java?rev=1385239&r1=1385238&r2=1385239&view=diff >> > >> ============================================================================== >> > --- jmeter/trunk/src/core/org/apache/jmeter/gui/action/Help.java >> (original) >> > +++ jmeter/trunk/src/core/org/apache/jmeter/gui/action/Help.java Sun Sep >> 16 10:06:30 2012 >> > @@ -29,6 +29,7 @@ import javax.swing.JDialog; >> > import javax.swing.JScrollPane; >> > >> > import org.apache.jmeter.gui.GuiPackage; >> > +import org.apache.jmeter.gui.util.EscapeDialog; >> > import org.apache.jmeter.swing.HtmlPane; >> > import org.apache.jmeter.util.JMeterUtils; >> > import org.apache.jorphan.gui.ComponentUtil; >> > @@ -69,7 +70,7 @@ public class Help implements Command { >> > */ >> > public void doAction(ActionEvent e) { >> > if (helpWindow == null) { >> > - helpWindow = new JDialog(new Frame(),// independent frame to >> > + helpWindow = new EscapeDialog(new Frame(),// independent >> frame to >> > // allow it to be >> overlaid >> > // by the main frame >> > JMeterUtils.getResString("help"),//$NON-NLS-1$ >> > >> > Modified: >> jmeter/trunk/src/core/org/apache/jmeter/gui/action/SearchTreeDialog.java >> > URL: >> http://svn.apache.org/viewvc/jmeter/trunk/src/core/org/apache/jmeter/gui/action/SearchTreeDialog.java?rev=1385239&r1=1385238&r2=1385239&view=diff >> > >> ============================================================================== >> > --- >> jmeter/trunk/src/core/org/apache/jmeter/gui/action/SearchTreeDialog.java >> (original) >> > +++ >> jmeter/trunk/src/core/org/apache/jmeter/gui/action/SearchTreeDialog.java >> Sun Sep 16 10:06:30 2012 >> > @@ -28,14 +28,19 @@ import java.util.Iterator; >> > import java.util.List; >> > import java.util.Set; >> > >> > +import javax.swing.AbstractAction; >> > +import javax.swing.Action; >> > +import javax.swing.ActionMap; >> > import javax.swing.BorderFactory; >> > import javax.swing.BoxLayout; >> > +import javax.swing.InputMap; >> > import javax.swing.JButton; >> > import javax.swing.JCheckBox; >> > import javax.swing.JComponent; >> > import javax.swing.JDialog; >> > import javax.swing.JFrame; >> > import javax.swing.JPanel; >> > +import javax.swing.JRootPane; >> > >> > import org.apache.commons.lang3.StringUtils; >> > import org.apache.jmeter.gui.GuiPackage; >> > @@ -73,29 +78,35 @@ public class SearchTreeDialog extends JD >> > * Store last search >> > */ >> > private transient String lastSearch = null; >> > - >> > - /** >> > - * Hide Window on ESC >> > - */ >> > - private transient ActionListener enterActionListener = new >> ActionListener() { >> > - public void actionPerformed(ActionEvent actionEvent) { >> > - doSearch(actionEvent); >> > - } >> > - }; >> > - >> > - /** >> > - * Do search on Enter >> > - */ >> > - private transient ActionListener escapeActionListener = new >> ActionListener() { >> > - public void actionPerformed(ActionEvent actionEvent) { >> > - setVisible(false); >> > - } >> > - }; >> > >> > public SearchTreeDialog() { >> > super((JFrame) null, >> JMeterUtils.getResString("search_tree_title"), true); //$NON-NLS-1$ >> > init(); >> > } >> > + >> > + @Override >> > + protected JRootPane createRootPane() { >> > + JRootPane rootPane = new JRootPane(); >> > + InputMap inputMap = >> rootPane.getInputMap(JComponent.WHEN_IN_FOCUSED_WINDOW); >> > + ActionMap actionMap = rootPane.getActionMap(); >> > + inputMap.put(KeyStrokes.ESC, "ESCAPE"); >> > + inputMap.put(KeyStrokes.ENTER, "ENTER"); >> > + // Hide Window on ESC >> > + Action escapeAction = new AbstractAction() { >> > + public void actionPerformed(ActionEvent actionEvent) { >> > + setVisible(false); >> > + } >> > + }; >> > + actionMap.put("ESCAPE", escapeAction); >> > + // Do search on Enter >> > + Action enterAction = new AbstractAction() { >> > + public void actionPerformed(ActionEvent actionEvent) { >> > + doSearch(actionEvent); >> > + } >> > + }; >> > + actionMap.put("ENTER", enterAction); >> > + return rootPane; >> > + } >> > >> > private void init() { >> > this.getContentPane().setLayout(new BorderLayout(10,10)); >> > @@ -129,8 +140,6 @@ public class SearchTreeDialog extends JD >> > buttonsPanel.add(cancelButton); >> > searchPanel.add(buttonsPanel, BorderLayout.SOUTH); >> > this.getContentPane().add(searchPanel); >> > - searchPanel.registerKeyboardAction(enterActionListener, >> KeyStrokes.ENTER, JComponent.WHEN_IN_FOCUSED_WINDOW); >> > - searchPanel.registerKeyboardAction(escapeActionListener, >> KeyStrokes.ESC, JComponent.WHEN_IN_FOCUSED_WINDOW); >> > searchTF.requestFocusInWindow(); >> > >> > this.pack(); >> > >> > Added: jmeter/trunk/src/core/org/apache/jmeter/gui/util/EscapeDialog.java >> > URL: >> http://svn.apache.org/viewvc/jmeter/trunk/src/core/org/apache/jmeter/gui/util/EscapeDialog.java?rev=1385239&view=auto >> > >> ============================================================================== >> > --- jmeter/trunk/src/core/org/apache/jmeter/gui/util/EscapeDialog.java >> (added) >> > +++ jmeter/trunk/src/core/org/apache/jmeter/gui/util/EscapeDialog.java >> Sun Sep 16 10:06:30 2012 >> > @@ -0,0 +1,63 @@ >> > +/* >> > + * Licensed to the Apache Software Foundation (ASF) under one or more >> > + * contributor license agreements. See the NOTICE file distributed with >> > + * this work for additional information regarding copyright ownership. >> > + * The ASF licenses this file to You under the Apache License, Version >> 2.0 >> > + * (the "License"); you may not use this file except in compliance with >> > + * the License. You may obtain a copy of the License at >> > + * >> > + * http://www.apache.org/licenses/LICENSE-2.0 >> > + * >> > + * Unless required by applicable law or agreed to in writing, software >> > + * distributed under the License is distributed on an "AS IS" BASIS, >> > + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or >> implied. >> > + * See the License for the specific language governing permissions and >> > + * limitations under the License. >> > + * >> > + */ >> > + >> > +package org.apache.jmeter.gui.util; >> > + >> > +import java.awt.Frame; >> > +import java.awt.event.ActionEvent; >> > + >> > +import javax.swing.AbstractAction; >> > +import javax.swing.Action; >> > +import javax.swing.InputMap; >> > +import javax.swing.JComponent; >> > +import javax.swing.JDialog; >> > +import javax.swing.JRootPane; >> > + >> > +import org.apache.jmeter.gui.action.KeyStrokes; >> > + >> > +/** >> > + * >> > + */ >> > +public class EscapeDialog extends JDialog { >> > + /** >> > + * >> > + */ >> > + private static final long serialVersionUID = 1319421816741139938L; >> > + >> > + public EscapeDialog(Frame frame, String title, boolean modal) { >> > + super(frame, title, modal); >> > + } >> > + >> > + protected JRootPane createRootPane() { >> > + JRootPane rootPane = new JRootPane(); >> > + Action actionListener = new AbstractAction() { >> > + /** >> > + * >> > + */ >> > + private static final long serialVersionUID = >> 2208129319916921772L; >> > + >> > + public void actionPerformed(ActionEvent e) { >> > + setVisible(false); >> > + } >> > + }; >> > + InputMap inputMap = >> rootPane.getInputMap(JComponent.WHEN_IN_FOCUSED_WINDOW); >> > + inputMap.put(KeyStrokes.ESC, "ESCAPE"); >> > + rootPane.getActionMap().put("ESCAPE", actionListener); >> > + return rootPane; >> > + } >> > +} >> > >> > Propchange: >> jmeter/trunk/src/core/org/apache/jmeter/gui/util/EscapeDialog.java >> > >> ------------------------------------------------------------------------------ >> > svn:mime-type = text/plain >> > >> > Modified: jmeter/trunk/xdocs/changes.xml >> > URL: >> http://svn.apache.org/viewvc/jmeter/trunk/xdocs/changes.xml?rev=1385239&r1=1385238&r2=1385239&view=diff >> > >> ============================================================================== >> > --- jmeter/trunk/xdocs/changes.xml (original) >> > +++ jmeter/trunk/xdocs/changes.xml Sun Sep 16 10:06:30 2012 >> > @@ -218,6 +218,7 @@ Cookie Manager has now the default HC3.1 >> > <li><bugzilla>53862</bugzilla> - Would be nice to have the JMeter >> Version available as a property</li> >> > <li><bugzilla>53806</bugzilla> - FileServer should provide direct >> access to the BufferedReader</li> >> > <li><bugzilla>53807</bugzilla> - CSV Dataset does not handle embedded >> new lines in quoted data</li> >> > +<li><bugzilla>53879</bugzilla> - GUI : Allow Popups to be closed with >> ESC key</li> >> > </ul> >> > >> > <h2>Non-functional changes</h2> >> > >> > >> > > > > -- > Cordialement. > Philippe Mouawad.
