From 4fc6139132c466c51c607f11836853843197b7e7 Mon Sep 17 00:00:00 2001 From: gmungoc Date: Thu, 28 Sep 2017 09:37:46 +0100 Subject: [PATCH] JAL-2744 refactor PopupMenu constructor to enable adding feature details --- src/jalview/gui/IdPanel.java | 5 +- src/jalview/gui/PopupMenu.java | 138 ++++++++++++++++++++--------------- src/jalview/gui/SeqPanel.java | 16 +--- test/jalview/gui/PopupMenuTest.java | 56 ++++++++++---- 4 files changed, 129 insertions(+), 86 deletions(-) diff --git a/src/jalview/gui/IdPanel.java b/src/jalview/gui/IdPanel.java index 3cc0ed3..f0aefb1 100755 --- a/src/jalview/gui/IdPanel.java +++ b/src/jalview/gui/IdPanel.java @@ -331,7 +331,8 @@ public class IdPanel extends JPanel * and any non-positional features */ List nlinks = Preferences.sequenceUrlLinks.getLinksForMenu(); - for (SequenceFeature sf : sq.getFeatures().getNonPositionalFeatures()) + List features = sq.getFeatures().getNonPositionalFeatures(); + for (SequenceFeature sf : features) { if (sf.links != null) { @@ -342,7 +343,7 @@ public class IdPanel extends JPanel } } - PopupMenu pop = new PopupMenu(alignPanel, sq, nlinks, + PopupMenu pop = new PopupMenu(alignPanel, sq, features, Preferences.getGroupURLLinks()); pop.show(this, e.getX(), e.getY()); } diff --git a/src/jalview/gui/PopupMenu.java b/src/jalview/gui/PopupMenu.java index 846ba64..21db7d7 100644 --- a/src/jalview/gui/PopupMenu.java +++ b/src/jalview/gui/PopupMenu.java @@ -34,7 +34,6 @@ import jalview.datamodel.Annotation; import jalview.datamodel.DBRefEntry; import jalview.datamodel.HiddenColumns; import jalview.datamodel.PDBEntry; -import jalview.datamodel.Sequence; import jalview.datamodel.SequenceFeature; import jalview.datamodel.SequenceGroup; import jalview.datamodel.SequenceI; @@ -176,25 +175,31 @@ public class PopupMenu extends JPopupMenu implements ColourChangeListener * Creates a new PopupMenu object. * * @param ap - * DOCUMENT ME! * @param seq - * DOCUMENT ME! + * @param features + * non-positional features (for seq not null), or positional features + * at residue (for seq equal to null) */ - public PopupMenu(final AlignmentPanel ap, Sequence seq, - List links) + public PopupMenu(final AlignmentPanel ap, SequenceI seq, + List features) { - this(ap, seq, links, null); + this(ap, seq, features, null); } /** + * Constructor * - * @param ap + * @param alignPanel * @param seq - * @param links + * the sequence under the cursor if in the Id panel, null if in the + * sequence panel + * @param features + * non-positional features if in the Id panel, features at the + * clicked residue if in the sequence panel * @param groupLinks */ - public PopupMenu(final AlignmentPanel ap, final SequenceI seq, - List links, List groupLinks) + public PopupMenu(final AlignmentPanel alignPanel, final SequenceI seq, + List features, List groupLinks) { // ///////////////////////////////////////////////////////// // If this is activated from the sequence panel, the user may want to @@ -202,7 +207,7 @@ public class PopupMenu extends JPopupMenu implements ColourChangeListener // // If from the IDPanel, we must display the sequence menu // //////////////////////////////////////////////////////// - this.ap = ap; + this.ap = alignPanel; sequence = seq; for (String ff : FileFormats.getInstance().getWritableFormats(true)) @@ -237,9 +242,9 @@ public class PopupMenu extends JPopupMenu implements ColourChangeListener /* * And repeat for the current selection group (if there is one): */ - final List selectedGroup = (ap.av.getSelectionGroup() == null + final List selectedGroup = (alignPanel.av.getSelectionGroup() == null ? Collections. emptyList() - : ap.av.getSelectionGroup().getSequences()); + : alignPanel.av.getSelectionGroup().getSequences()); buildAnnotationTypesMenus(groupShowAnnotationsMenu, groupHideAnnotationsMenu, selectedGroup); configureReferenceAnnotationsMenu(groupAddReferenceAnnotations, @@ -257,7 +262,7 @@ public class PopupMenu extends JPopupMenu implements ColourChangeListener if (seq != null) { sequenceMenu.setText(sequence.getName()); - if (seq == ap.av.getAlignment().getSeqrep()) + if (seq == alignPanel.av.getAlignment().getSeqrep()) { makeReferenceSeq.setText( MessageManager.getString("action.unmark_as_reference")); @@ -268,7 +273,7 @@ public class PopupMenu extends JPopupMenu implements ColourChangeListener MessageManager.getString("action.set_as_reference")); } - if (!ap.av.getAlignment().isNucleotide()) + if (!alignPanel.av.getAlignment().isNucleotide()) { remove(rnaStructureMenu); } @@ -279,7 +284,7 @@ public class PopupMenu extends JPopupMenu implements ColourChangeListener * add menu items to 2D-render any alignment or sequence secondary * structure annotation */ - AlignmentAnnotation[] aas = ap.av.getAlignment() + AlignmentAnnotation[] aas = alignPanel.av.getAlignment() .getAlignmentAnnotation(); if (aas != null) { @@ -299,7 +304,7 @@ public class PopupMenu extends JPopupMenu implements ColourChangeListener @Override public void actionPerformed(ActionEvent e) { - new AppVarna(seq, aa, ap); + new AppVarna(seq, aa, alignPanel); } }); rnaStructureMenu.add(menuItem); @@ -328,7 +333,7 @@ public class PopupMenu extends JPopupMenu implements ColourChangeListener public void actionPerformed(ActionEvent e) { // TODO: VARNA does'nt print gaps in the sequence - new AppVarna(seq, aa, ap); + new AppVarna(seq, aa, alignPanel); } }); rnaStructureMenu.add(menuItem); @@ -353,8 +358,8 @@ public class PopupMenu extends JPopupMenu implements ColourChangeListener }); add(menuItem); - if (ap.av.getSelectionGroup() != null - && ap.av.getSelectionGroup().getSize() > 1) + if (alignPanel.av.getSelectionGroup() != null + && alignPanel.av.getSelectionGroup().getSize() > 1) { menuItem = new JMenuItem(MessageManager .formatMessage("label.represent_group_with", new Object[] @@ -370,12 +375,12 @@ public class PopupMenu extends JPopupMenu implements ColourChangeListener sequenceMenu.add(menuItem); } - if (ap.av.hasHiddenRows()) + if (alignPanel.av.hasHiddenRows()) { - final int index = ap.av.getAlignment().findIndex(seq); + final int index = alignPanel.av.getAlignment().findIndex(seq); - if (ap.av.adjustForHiddenSeqs(index) - - ap.av.adjustForHiddenSeqs(index - 1) > 1) + if (alignPanel.av.adjustForHiddenSeqs(index) + - alignPanel.av.adjustForHiddenSeqs(index - 1) > 1) { menuItem = new JMenuItem( MessageManager.getString("action.reveal_sequences")); @@ -384,10 +389,10 @@ public class PopupMenu extends JPopupMenu implements ColourChangeListener @Override public void actionPerformed(ActionEvent e) { - ap.av.showSequence(index); - if (ap.overviewPanel != null) + alignPanel.av.showSequence(index); + if (alignPanel.overviewPanel != null) { - ap.overviewPanel.updateOverviewImage(); + alignPanel.overviewPanel.updateOverviewImage(); } } }); @@ -396,7 +401,7 @@ public class PopupMenu extends JPopupMenu implements ColourChangeListener } } // for the case when no sequences are even visible - if (ap.av.hasHiddenRows()) + if (alignPanel.av.hasHiddenRows()) { { menuItem = new JMenuItem( @@ -406,10 +411,10 @@ public class PopupMenu extends JPopupMenu implements ColourChangeListener @Override public void actionPerformed(ActionEvent e) { - ap.av.showAllHiddenSeqs(); - if (ap.overviewPanel != null) + alignPanel.av.showAllHiddenSeqs(); + if (alignPanel.overviewPanel != null) { - ap.overviewPanel.updateOverviewImage(); + alignPanel.overviewPanel.updateOverviewImage(); } } }); @@ -418,9 +423,9 @@ public class PopupMenu extends JPopupMenu implements ColourChangeListener } } - SequenceGroup sg = ap.av.getSelectionGroup(); + SequenceGroup sg = alignPanel.av.getSelectionGroup(); boolean isDefinedGroup = (sg != null) - ? ap.av.getAlignment().getGroups().contains(sg) + ? alignPanel.av.getAlignment().getGroups().contains(sg) : false; if (sg != null && sg.getSize() > 0) @@ -458,7 +463,7 @@ public class PopupMenu extends JPopupMenu implements ColourChangeListener Hashtable pdbe = new Hashtable<>(), reppdb = new Hashtable<>(); SequenceI sqass = null; - for (SequenceI sq : ap.av.getSequenceSelection()) + for (SequenceI sq : alignPanel.av.getSequenceSelection()) { Vector pes = sq.getDatasetSequence().getAllPDBEntries(); if (pes != null && pes.size() > 0) @@ -508,24 +513,50 @@ public class PopupMenu extends JPopupMenu implements ColourChangeListener rnaStructureMenu.setVisible(false); } - if (links != null && links.size() > 0) - { - addFeatureLinks(seq, links); - } + addLinks(seq, features); } /** * Adds a 'Link' menu item with a sub-menu item for each hyperlink provided. + * When seq is not null, these are links for the sequence id, which may be to + * external web sites for the sequence accession, and/or links embedded in + * non-positional features. When seq is null, only links embedded in the + * provided features are added. * * @param seq - * @param links + * @param features */ - void addFeatureLinks(final SequenceI seq, List links) + void addLinks(final SequenceI seq, List features) { JMenu linkMenu = new JMenu(MessageManager.getString("action.link")); + + List nlinks = null; + if (seq != null) + { + nlinks = Preferences.sequenceUrlLinks.getLinksForMenu(); + } + else + { + nlinks = new ArrayList<>(); + } + + if (features != null) + { + for (SequenceFeature sf : features) + { + if (sf.links != null) + { + for (String link : sf.links) + { + nlinks.add(link); + } + } + } + } + Map> linkset = new LinkedHashMap<>(); - for (String link : links) + for (String link : nlinks) { UrlLink urlLink = null; try @@ -548,25 +579,18 @@ public class PopupMenu extends JPopupMenu implements ColourChangeListener addshowLinks(linkMenu, linkset.values()); - // disable link menu if there are no valid entries + // only add link menu if it has entries if (linkMenu.getItemCount() > 0) { - linkMenu.setEnabled(true); - } - else - { - linkMenu.setEnabled(false); - } - - if (sequence != null) - { - sequenceMenu.add(linkMenu); - } - else - { - add(linkMenu); + if (sequence != null) + { + sequenceMenu.add(linkMenu); + } + else + { + add(linkMenu); + } } - } /** diff --git a/src/jalview/gui/SeqPanel.java b/src/jalview/gui/SeqPanel.java index e99e577..ad80a3e 100644 --- a/src/jalview/gui/SeqPanel.java +++ b/src/jalview/gui/SeqPanel.java @@ -59,7 +59,6 @@ import java.awt.event.MouseListener; import java.awt.event.MouseMotionListener; import java.awt.event.MouseWheelEvent; import java.awt.event.MouseWheelListener; -import java.util.ArrayList; import java.util.Collections; import java.util.List; @@ -1782,21 +1781,10 @@ public class SeqPanel extends JPanel final int column = findColumn(evt); final int seq = findSeq(evt); SequenceI sequence = av.getAlignment().getSequenceAt(seq); - List allFeatures = ap.getFeatureRenderer() + List features = ap.getFeatureRenderer() .findFeaturesAtColumn(sequence, column + 1); - List links = new ArrayList<>(); - for (SequenceFeature sf : allFeatures) - { - if (sf.links != null) - { - for (String link : sf.links) - { - links.add(link); - } - } - } - PopupMenu pop = new PopupMenu(ap, null, links); + PopupMenu pop = new PopupMenu(ap, null, features); pop.show(this, evt.getX(), evt.getY()); } diff --git a/test/jalview/gui/PopupMenuTest.java b/test/jalview/gui/PopupMenuTest.java index 335240b..40e624d 100644 --- a/test/jalview/gui/PopupMenuTest.java +++ b/test/jalview/gui/PopupMenuTest.java @@ -26,21 +26,26 @@ import static org.testng.AssertJUnit.assertEquals; import static org.testng.AssertJUnit.assertFalse; import static org.testng.AssertJUnit.assertTrue; +import jalview.bin.Cache; import jalview.datamodel.AlignmentAnnotation; import jalview.datamodel.AlignmentI; import jalview.datamodel.Annotation; import jalview.datamodel.DBRefEntry; import jalview.datamodel.DBRefSource; -import jalview.datamodel.Sequence; +import jalview.datamodel.SequenceFeature; import jalview.datamodel.SequenceI; import jalview.io.DataSourceType; import jalview.io.FileFormat; import jalview.io.FormatAdapter; +import jalview.urls.api.UrlProviderFactoryI; +import jalview.urls.desktop.DesktopUrlProviderFactory; import jalview.util.MessageManager; +import jalview.util.UrlConstants; import java.awt.Component; import java.io.IOException; import java.util.ArrayList; +import java.util.Collections; import java.util.List; import javax.swing.JMenu; @@ -80,6 +85,25 @@ public class PopupMenuTest @BeforeMethod(alwaysRun = true) public void setUp() throws IOException { + Cache.loadProperties("test/jalview/io/testProps.jvprops"); + String inMenuString = ("EMBL-EBI Search | http://www.ebi.ac.uk/ebisearch/search.ebi?db=allebi&query=$" + + SEQUENCE_ID + + "$" + + "|" + + "UNIPROT | http://www.uniprot.org/uniprot/$" + DB_ACCESSION + "$") + + "|" + + ("INTERPRO | http://www.ebi.ac.uk/interpro/entry/$" + + DB_ACCESSION + "$") + + "|" + + + // Gene3D entry tests for case (in)sensitivity + ("Gene3D | http://gene3d.biochem.ucl.ac.uk/Gene3D/search?sterm=$" + + DB_ACCESSION + "$&mode=protein"); + + UrlProviderFactoryI factory = new DesktopUrlProviderFactory( + UrlConstants.DEFAULT_LABEL, inMenuString, ""); + Preferences.sequenceUrlLinks = factory.createUrlProvider(); + alignment = new FormatAdapter().readFile(TEST_DATA, DataSourceType.PASTE, FileFormat.Fasta); AlignFrame af = new AlignFrame(alignment, 700, 500); @@ -495,17 +519,19 @@ public class PopupMenuTest // add all the dbrefs to the sequences: Uniprot 1 each, Interpro all 3 to // seq0, Gene3D to seq1 - seqs.get(0).addDBRef(refs.get(0)); + SequenceI seq = seqs.get(0); + seq.addDBRef(refs.get(0)); - seqs.get(0).addDBRef(refs.get(1)); - seqs.get(0).addDBRef(refs.get(2)); - seqs.get(0).addDBRef(refs.get(3)); + seq.addDBRef(refs.get(1)); + seq.addDBRef(refs.get(2)); + seq.addDBRef(refs.get(3)); seqs.get(1).addDBRef(refs.get(4)); seqs.get(1).addDBRef(refs.get(5)); // get the Popup Menu for first sequence - testee = new PopupMenu(parentPanel, (Sequence) seqs.get(0), links); + List noFeatures = Collections. emptyList(); + testee = new PopupMenu(parentPanel, seq, noFeatures); Component[] seqItems = testee.sequenceMenu.getMenuComponents(); JMenu linkMenu = (JMenu) seqItems[6]; Component[] linkItems = linkMenu.getMenuComponents(); @@ -519,15 +545,18 @@ public class PopupMenuTest // sequence id for each link should match corresponding DB accession id for (int i = 1; i < 4; i++) { - assertEquals(refs.get(i - 1).getSource(), ((JMenuItem) linkItems[i]) + String msg = seq.getName() + " link[" + i + "]"; + assertEquals(msg, refs.get(i - 1).getSource(), + ((JMenuItem) linkItems[i]) .getText().split("\\|")[0]); - assertEquals(refs.get(i - 1).getAccessionId(), + assertEquals(msg, refs.get(i - 1).getAccessionId(), ((JMenuItem) linkItems[i]) .getText().split("\\|")[1]); } // get the Popup Menu for second sequence - testee = new PopupMenu(parentPanel, (Sequence) seqs.get(1), links); + seq = seqs.get(1); + testee = new PopupMenu(parentPanel, seq, noFeatures); seqItems = testee.sequenceMenu.getMenuComponents(); linkMenu = (JMenu) seqItems[6]; linkItems = linkMenu.getMenuComponents(); @@ -541,9 +570,11 @@ public class PopupMenuTest // sequence id for each link should match corresponding DB accession id for (int i = 1; i < 3; i++) { - assertEquals(refs.get(i + 3).getSource(), ((JMenuItem) linkItems[i]) + String msg = seq.getName() + " link[" + i + "]"; + assertEquals(msg, refs.get(i + 3).getSource(), + ((JMenuItem) linkItems[i]) .getText().split("\\|")[0].toUpperCase()); - assertEquals(refs.get(i + 3).getAccessionId(), + assertEquals(msg, refs.get(i + 3).getAccessionId(), ((JMenuItem) linkItems[i]).getText().split("\\|")[1]); } @@ -552,8 +583,7 @@ public class PopupMenuTest nomatchlinks.add("NOMATCH | http://www.uniprot.org/uniprot/$" + DB_ACCESSION + "$"); - testee = new PopupMenu(parentPanel, (Sequence) seqs.get(0), - nomatchlinks); + testee = new PopupMenu(parentPanel, seq, noFeatures); seqItems = testee.sequenceMenu.getMenuComponents(); linkMenu = (JMenu) seqItems[6]; assertFalse(linkMenu.isEnabled()); -- 1.7.10.2