From d156987a513b1da92fd6fbf7678b4a8e7ffc8d08 Mon Sep 17 00:00:00 2001 From: gmungoc Date: Thu, 12 Apr 2018 14:01:26 +0100 Subject: [PATCH] JAL-2944 tidies for review comments --- resources/lang/Messages.properties | 5 - resources/lang/Messages_es.properties | 5 - .../api/structures/JalviewStructureDisplayI.java | 26 ++- src/jalview/gui/AppJmol.java | 3 +- src/jalview/gui/ChimeraViewFrame.java | 1 - src/jalview/gui/Desktop.java | 22 +- src/jalview/gui/JDatabaseTree.java | 17 +- src/jalview/gui/SequenceFetcher.java | 14 +- src/jalview/gui/StructureChooser.java | 173 +++++++++------- src/jalview/gui/StructureViewer.java | 5 +- src/jalview/gui/StructureViewerBase.java | 63 ++---- src/jalview/jbgui/GStructureChooser.java | 218 +++++++++----------- .../structure/StructureSelectionManager.java | 5 +- test/jalview/gui/StructureChooserTest.java | 11 +- 14 files changed, 297 insertions(+), 271 deletions(-) diff --git a/resources/lang/Messages.properties b/resources/lang/Messages.properties index f526699..a80ac17 100644 --- a/resources/lang/Messages.properties +++ b/resources/lang/Messages.properties @@ -400,10 +400,6 @@ label.view_name_original = Original label.enter_view_name = Enter View Name label.enter_label = Enter label label.enter_label_for_the_structure = Enter a label for the structure -label.pdb_entry_is_already_displayed = {0} is already displayed.\nDo you want to re-use this viewer ? -label.map_sequences_to_visible_window = Map Sequences to Visible Window: {0} -label.add_pdbentry_to_view = Do you want to add {0} to the view called\n{1}\n -label.align_to_existing_structure_view = Align to existing structure view label.pdb_entries_couldnt_be_retrieved = The following pdb entries could not be retrieved from the PDB\:\n{0}\nPlease retry, or try downloading them manually. label.couldnt_load_file = Couldn't load file label.couldnt_find_pdb_id_in_file = Couldn't find a PDB id in the file supplied. Please enter an Id to identify this structure. @@ -1213,7 +1209,6 @@ label.pdb_sequence_fetcher = PDB Sequence Fetcher label.result = result label.results = results label.structure_chooser = Structure Chooser -label.select = Select : label.invert = Invert label.select_pdb_file = Select PDB File info.select_filter_option = Select Filter Option/Manual Entry diff --git a/resources/lang/Messages_es.properties b/resources/lang/Messages_es.properties index 77f053e..61bf42a 100644 --- a/resources/lang/Messages_es.properties +++ b/resources/lang/Messages_es.properties @@ -367,10 +367,6 @@ label.ignore_unmatched_dropped_files = Ignorar los ficheros sin coincidencias? label.enter_view_name = Introduzca un nombre para la vista label.enter_label = Introducir etiqueta label.enter_label_for_the_structure = Introducir una etiqueta para la estructura -label.pdb_entry_is_already_displayed = {0} Ya est\u00E1 mostrado.\nQuieres volver a usar este visor? -label.map_sequences_to_visible_window = Mapa de secuencias en ventana visible: {0} -label.add_pdbentry_to_view = Quieres a\u00F1adir {0} a la vista llamada\n{1}\n -label.align_to_existing_structure_view = Alinear a una estructura ya existente label.pdb_entries_couldnt_be_retrieved = Las siguientes entradas pdb no pueden ser extra\u00EDdas del PDB\:\n{0}\nPor favor, prueba descarg\u00E1ndolas manualmente. label.couldnt_load_file = No se pudo cargar el fichero label.couldnt_find_pdb_id_in_file = No se pudo encontrar un Id PDB en el fichero suministrado. Por favor, introduzca un Id para identificar esta estructura. @@ -1172,7 +1168,6 @@ label.structures_filter=Filtro de Estructuras label.scale_protein_to_cdna=Adaptar proteína a cDNA label.scale_protein_to_cdna_tip=Hacer a los residuos de proteínas de la misma anchura que los codones en ventanas divididas status.loading_cached_pdb_entries=Cargando Entradas PDB en Caché -label.select=Seleccionar : label.select_by_annotation=Seleccionar/Ocultar Columnas por Anotación action.select_by_annotation=Seleccionar/Ocultar Columnas por Anotación... action.export_features=Exportar Características diff --git a/src/jalview/api/structures/JalviewStructureDisplayI.java b/src/jalview/api/structures/JalviewStructureDisplayI.java index a27cec6..9ba513a 100644 --- a/src/jalview/api/structures/JalviewStructureDisplayI.java +++ b/src/jalview/api/structures/JalviewStructureDisplayI.java @@ -72,12 +72,32 @@ public interface JalviewStructureDisplayI */ boolean hasMapping(); - // construction method - move to another interface ? + /** + * Checks if the PDB file is already loaded in this viewer, if so just adds + * mappings as necessary and answers true, else answers false. This supports + * the use case of adding additional chains of the same structure to a viewer. + * + * @param seq + * @param chains + * @param apanel + * @param pdbId + * @return + */ boolean addAlreadyLoadedFile(SequenceI[] seq, String[] chains, AlignmentViewPanel apanel, String pdbId); - // construction method - move to another interface ? - boolean addToExistingViewer(PDBEntry pdbentry, SequenceI[] seq, + /** + * Adds one or more chains (sequences) of a PDB structure to this structure + * viewer + * + * @param pdbentry + * @param seq + * @param chains + * @param apanel + * @param pdbId + * @return + */ + void addToExistingViewer(PDBEntry pdbentry, SequenceI[] seq, String[] chains, AlignmentViewPanel apanel, String pdbId); /** diff --git a/src/jalview/gui/AppJmol.java b/src/jalview/gui/AppJmol.java index 7b4af56..dae54a5 100644 --- a/src/jalview/gui/AppJmol.java +++ b/src/jalview/gui/AppJmol.java @@ -259,8 +259,6 @@ public class AppJmol extends StructureViewerBase jmb.setFinishedInit(true); } - boolean allChainsSelected = false; - @Override void showSelectedChains() { @@ -466,6 +464,7 @@ public class AppJmol extends StructureViewerBase String file = jmb.getPdbEntry(pi).getFile(); if (file == null) { + // todo: extract block as method and pull up (also ChimeraViewFrame) // retrieve the pdb and store it locally AlignmentI pdbseq = null; pdbid = jmb.getPdbEntry(pi).getId(); diff --git a/src/jalview/gui/ChimeraViewFrame.java b/src/jalview/gui/ChimeraViewFrame.java index b4520c4..6342cf0 100644 --- a/src/jalview/gui/ChimeraViewFrame.java +++ b/src/jalview/gui/ChimeraViewFrame.java @@ -635,7 +635,6 @@ public class ChimeraViewFrame extends StructureViewerBase private String fetchPdbFile(PDBEntry processingEntry) throws Exception { - // FIXME: this is duplicated code with Jmol frame ? String filePath = null; Pdb pdbclient = new Pdb(); AlignmentI pdbseq = null; diff --git a/src/jalview/gui/Desktop.java b/src/jalview/gui/Desktop.java index 4fd478b..9a696e9 100644 --- a/src/jalview/gui/Desktop.java +++ b/src/jalview/gui/Desktop.java @@ -3400,6 +3400,18 @@ public class Desktop extends jalview.jbgui.GDesktop Cache.setProperty(EXPERIMENTAL_FEATURES, Boolean.toString(selected)); } + /** + * Answers a (possibly empty) list of any structure viewer frames (currently + * for either Jmol or Chimera) which are currently open. This may optionally + * be restricted to viewers of a specified class, or viewers linked to a + * specified alignment panel. + * + * @param apanel + * if not null, only return viewers linked to this panel + * @param structureViewerClass + * if not null, only return viewers of this class + * @return + */ public List getStructureViewers( AlignmentPanel apanel, Class structureViewerClass) @@ -3414,11 +3426,11 @@ public class Desktop extends jalview.jbgui.GDesktop if (structureViewerClass == null || structureViewerClass.isInstance(frame)) { - if (apanel == null - || ((StructureViewerBase) frame).isLinkedWith(apanel)) - { - result.add((StructureViewerBase) frame); - } + if (apanel == null + || ((StructureViewerBase) frame).isLinkedWith(apanel)) + { + result.add((StructureViewerBase) frame); + } } } } diff --git a/src/jalview/gui/JDatabaseTree.java b/src/jalview/gui/JDatabaseTree.java index 0a6b9d6..1018d6e 100644 --- a/src/jalview/gui/JDatabaseTree.java +++ b/src/jalview/gui/JDatabaseTree.java @@ -101,10 +101,10 @@ public class JDatabaseTree extends JalviewDialog implements KeyListener * identical DB sources, and should be collapsed. */ DefaultMutableTreeNode tn = null, root = new DefaultMutableTreeNode(); - Hashtable source = new Hashtable(); + Hashtable source = new Hashtable<>(); sfetcher = sfetch; String dbs[] = sfetch.getSupportedDb(); - Hashtable ht = new Hashtable(); + Hashtable ht = new Hashtable<>(); for (int i = 0; i < dbs.length; i++) { tn = source.get(dbs[i]); @@ -370,7 +370,7 @@ public class JDatabaseTree extends JalviewDialog implements KeyListener tsel = dbviews.getSelectionPaths(); boolean forcedFirstChild = false; - List srcs = new ArrayList(); + List srcs = new ArrayList<>(); if (tsel != null) { for (TreePath tp : tsel) @@ -489,7 +489,7 @@ public class JDatabaseTree extends JalviewDialog implements KeyListener return null; } StringBuffer sb = new StringBuffer(); - HashSet hs = new HashSet(); + HashSet hs = new HashSet<>(); for (DbSourceProxy dbs : getSelectedSources()) { String tq = dbs.getTestQuery(); @@ -506,7 +506,7 @@ public class JDatabaseTree extends JalviewDialog implements KeyListener return sb.toString(); } - List lstners = new Vector(); + List lstners = new Vector<>(); public void addActionListener(ActionListener actionListener) { @@ -596,4 +596,11 @@ public class JDatabaseTree extends JalviewDialog implements KeyListener // TODO Auto-generated method stub } + + @Override + public void setVisible(boolean arg0) + { + System.out.println("setVisible: " + arg0); + super.setVisible(arg0); + } } diff --git a/src/jalview/gui/SequenceFetcher.java b/src/jalview/gui/SequenceFetcher.java index 8d46792..f545e70 100755 --- a/src/jalview/gui/SequenceFetcher.java +++ b/src/jalview/gui/SequenceFetcher.java @@ -273,7 +273,7 @@ public class SequenceFetcher extends JPanel implements Runnable return Collections.emptyList(); } } - sf.newAlframes = new ArrayList(); + sf.newAlframes = new ArrayList<>(); sf.run(); return sf.newAlframes; } @@ -674,10 +674,10 @@ public class SequenceFetcher extends JPanel implements Runnable // TODO: Refactor to GUI independent code and write tests. // indicate if successive sources should be merged into one alignment. boolean addToLast = false; - List aresultq = new ArrayList(); - List presultTitle = new ArrayList(); - List presult = new ArrayList(); - List aresult = new ArrayList(); + List aresultq = new ArrayList<>(); + List presultTitle = new ArrayList<>(); + List presult = new ArrayList<>(); + List aresult = new ArrayList<>(); Iterator proxies = database.getSelectedSources() .iterator(); String[] qries; @@ -695,7 +695,7 @@ public class SequenceFetcher extends JPanel implements Runnable nqueries = nextFetch.size(); // save the remaining queries in the original array qries = nextFetch.toArray(new String[nqueries]); - nextFetch = new ArrayList(); + nextFetch = new ArrayList<>(); } DbSourceProxy proxy = proxies.next(); @@ -861,7 +861,7 @@ public class SequenceFetcher extends JPanel implements Runnable List aresult, List nextFetch) throws Exception { StringBuilder multiacc = new StringBuilder(); - List tosend = new ArrayList(); + List tosend = new ArrayList<>(); while (accessions.hasNext()) { String nel = accessions.next(); diff --git a/src/jalview/gui/StructureChooser.java b/src/jalview/gui/StructureChooser.java index cd1cfc3..e18d6af 100644 --- a/src/jalview/gui/StructureChooser.java +++ b/src/jalview/gui/StructureChooser.java @@ -69,6 +69,8 @@ import javax.swing.table.AbstractTableModel; public class StructureChooser extends GStructureChooser implements IProgressIndicator { + private static final String AUTOSUPERIMPOSE = "AUTOSUPERIMPOSE"; + private static int MAX_QLENGTH = 7820; private SequenceI selectedSequence; @@ -89,6 +91,8 @@ public class StructureChooser extends GStructureChooser private boolean cachedPDBExists; + private static StructureViewer lastTargetedView = null; + public StructureChooser(SequenceI[] selectedSeqs, SequenceI selectedSeq, AlignmentPanel ap) { @@ -102,14 +106,14 @@ public class StructureChooser extends GStructureChooser /** * Initializes parameters used by the Structure Chooser Panel */ - public void init() + protected void init() { if (!Jalview.isHeadlessMode()) { progressBar = new ProgressBar(this.statusPanel, this.statusBar); } - chk_superpose.setSelected(Cache.getDefault("AUTOSUPERIMPOSE", true)); + chk_superpose.setSelected(Cache.getDefault(AUTOSUPERIMPOSE, true)); // ensure a filter option is in force for search populateFilterComboBox(true, cachedPDBExists); @@ -137,6 +141,11 @@ public class StructureChooser extends GStructureChooser discoverPDBStructuresThread.start(); } + /** + * Builds a drop-down choice list of existing structure viewers to which new + * structures may be added. If this list is empty then it, and the 'Add' + * button, are hidden. + */ private void discoverStructureViews() { if (Desktop.instance != null) @@ -164,22 +173,27 @@ public class StructureChooser extends GStructureChooser targetView.addItem(viewHandler); } } - targetView.setVisible(targetView.getItemCount() > 0); - btn_view.setVisible(targetView.isVisible()); - if (targetView.isVisible()) { - // finally, restore last targeted view by default. + + /* + * show option to Add to viewer if at least 1 viewer found + */ + targetView.setVisible(false); + if (targetView.getItemCount() > 0) + { + targetView.setVisible(true); if (lastTargetedView != null) { targetView.setSelectedItem(lastTargetedView); - } else { + } + else + { targetView.setSelectedIndex(0); } } + btn_add.setVisible(targetView.isVisible()); } } - private static StructureViewer lastTargetedView = null; - /** * Updates the progress indicator with the specified message * @@ -188,7 +202,7 @@ public class StructureChooser extends GStructureChooser * @param id * unique handle for this indicator */ - public void updateProgressIndicator(String message, long id) + protected void updateProgressIndicator(String message, long id) { if (progressIndicator != null) { @@ -200,7 +214,7 @@ public class StructureChooser extends GStructureChooser * Retrieve meta-data for all the structure(s) for a given sequence(s) in a * selection group */ - public void fetchStructuresMetaData() + void fetchStructuresMetaData() { long startTime = System.currentTimeMillis(); pdbRestCleint = PDBFTSRestClient.getInstance(); @@ -271,7 +285,7 @@ public class StructureChooser extends GStructureChooser } } - public void loadLocalCachedPDBEntries() + protected void loadLocalCachedPDBEntries() { ArrayList entries = new ArrayList<>(); for (SequenceI seq : selectedSequences) @@ -302,7 +316,7 @@ public class StructureChooser extends GStructureChooser * @return the built query string */ - public static String buildQuery(SequenceI seq) + static String buildQuery(SequenceI seq) { boolean isPDBRefsFound = false; boolean isUniProtRefsFound = false; @@ -404,7 +418,7 @@ public class StructureChooser extends GStructureChooser * @param seqName * @return */ - public static boolean isValidSeqName(String seqName) + static boolean isValidSeqName(String seqName) { // System.out.println("seqName : " + seqName); String ignoreList = "pdb,uniprot,swiss-prot"; @@ -427,7 +441,7 @@ public class StructureChooser extends GStructureChooser return true; } - public static String getDBRefId(DBRefEntry dbRef) + static String getDBRefId(DBRefEntry dbRef) { String ref = dbRef.getAccessionId().replaceAll("GO:", ""); return ref; @@ -439,7 +453,7 @@ public class StructureChooser extends GStructureChooser * @param fieldToFilterBy * the field to filter by */ - public void filterResultSet(final String fieldToFilterBy) + void filterResultSet(final String fieldToFilterBy) { Thread filterThread = new Thread(new Runnable() { @@ -549,7 +563,7 @@ public class StructureChooser extends GStructureChooser * Handles action event for btn_pdbFromFile */ @Override - public void pdbFromFile_actionPerformed() + protected void pdbFromFile_actionPerformed() { jalview.io.JalviewFileChooser chooser = new jalview.io.JalviewFileChooser( jalview.bin.Cache.getProperty("LAST_DIRECTORY")); @@ -650,28 +664,37 @@ public class StructureChooser extends GStructureChooser } /** - * Validates user selection and activates the view button if all parameters - * are correct + * Validates user selection and enables the 'Add' and 'New View' buttons if + * all parameters are correct (the Add button will only be visible if there is + * at least one existing structure viewer open). This basically means at least + * one structure selected and no error messages. + *

+ * The 'Superpose Structures' option is enabled if either more than one + * structure is selected, or the 'Add' to existing view option is enabled, and + * disabled if the only option is to open a new view of a single structure. */ @Override - public void validateSelections() + protected void validateSelections() { FilterOption selectedFilterOpt = ((FilterOption) cmb_filterOption .getSelectedItem()); - btn_view.setEnabled(false); + btn_add.setEnabled(false); String currentView = selectedFilterOpt.getView(); + int selectedCount = 0; if (currentView == VIEWS_FILTER) { - if (getResultTable().getSelectedRows().length > 0) + selectedCount = getResultTable().getSelectedRows().length; + if (selectedCount > 0) { - btn_view.setEnabled(true); + btn_add.setEnabled(true); } } else if (currentView == VIEWS_LOCAL_PDB) { - if (tbl_local_pdb.getSelectedRows().length > 0) + selectedCount = tbl_local_pdb.getSelectedRows().length; + if (selectedCount > 0) { - btn_view.setEnabled(true); + btn_add.setEnabled(true); } } else if (currentView == VIEWS_ENTER_ID) @@ -682,12 +705,21 @@ public class StructureChooser extends GStructureChooser { validateAssociationFromFile(); } + + btn_newView.setEnabled(btn_add.isEnabled()); + + /* + * enable 'Superpose' option if more than one structure is selected, + * or there are view(s) available to add structure(s) to + */ + chk_superpose + .setEnabled(selectedCount > 1 || targetView.getItemCount() > 0); } /** * Validates inputs from the Manual PDB entry panel */ - public void validateAssociationEnterPdb() + protected void validateAssociationEnterPdb() { AssociateSeqOptions assSeqOpt = (AssociateSeqOptions) idInputAssSeqPanel .getCmb_assSeq().getSelectedItem(); @@ -713,7 +745,7 @@ public class StructureChooser extends GStructureChooser txt_search.setEnabled(true); if (isValidPBDEntry) { - btn_view.setEnabled(true); + btn_add.setEnabled(true); lbl_pdbManualFetchStatus.setToolTipText(""); lbl_pdbManualFetchStatus.setIcon(goodImage); } @@ -728,7 +760,7 @@ public class StructureChooser extends GStructureChooser /** * Validates inputs for the manual PDB file selection options */ - public void validateAssociationFromFile() + protected void validateAssociationFromFile() { AssociateSeqOptions assSeqOpt = (AssociateSeqOptions) fileChooserAssSeqPanel .getCmb_assSeq().getSelectedItem(); @@ -739,7 +771,7 @@ public class StructureChooser extends GStructureChooser btn_pdbFromFile.setEnabled(true); if (selectedPdbFileName != null && selectedPdbFileName.length() > 0) { - btn_view.setEnabled(true); + btn_add.setEnabled(true); lbl_fromFileStatus.setIcon(goodImage); } } @@ -751,7 +783,7 @@ public class StructureChooser extends GStructureChooser } @Override - public void cmbAssSeqStateChanged() + protected void cmbAssSeqStateChanged() { validateSelections(); } @@ -815,21 +847,22 @@ public class StructureChooser extends GStructureChooser } return found; } + /** - * Handles action event for btn_ok + * Handles the 'New View' action */ @Override - public void newview_ActionPerformed() + protected void newView_ActionPerformed() { targetView.setSelectedItem(null); showStructures(false); } /** - * Handles action event for btn_ok + * Handles the 'Add to existing viewer' action */ @Override - public void view_ActionPerformed() + protected void add_ActionPerformed() { showStructures(false); } @@ -1015,24 +1048,29 @@ public class StructureChooser extends GStructureChooser } /** + * Answers a structure viewer (new or existing) configured to superimpose + * added structures or not according to the user's choice + * * @param ssm - * @return targetted structure view (new or existing) configured according to - * superpose checkbox + * @return */ - public StructureViewer getTargetedStructureViewer( + StructureViewer getTargetedStructureViewer( StructureSelectionManager ssm) { - Object _sv = targetView.getSelectedItem(); - StructureViewer sv; - if (_sv == null) - { - sv = new StructureViewer(ssm); - } else { - sv = (StructureViewer) _sv; - } - sv.setSuperpose(chk_superpose.isSelected()); - return sv; + Object sv = targetView.getSelectedItem(); + + return sv == null ? new StructureViewer(ssm) : (StructureViewer) sv; } + + /** + * Adds PDB structures to a new or existing structure viewer + * + * @param ssm + * @param pdbEntriesToView + * @param alignPanel + * @param sequences + * @return + */ private StructureViewer launchStructureViewer( StructureSelectionManager ssm, final PDBEntry[] pdbEntriesToView, @@ -1041,8 +1079,15 @@ public class StructureChooser extends GStructureChooser long progressId = sequences.hashCode(); setProgressBar(MessageManager .getString("status.launching_3d_structure_viewer"), progressId); - final StructureViewer sViewer = getTargetedStructureViewer(ssm); - sViewer.setSuperpose(chk_superpose.isSelected()); + final StructureViewer theViewer = getTargetedStructureViewer(ssm); + boolean superimpose = chk_superpose.isSelected(); + theViewer.setSuperpose(superimpose); + + /* + * remember user's choice of superimpose or not + */ + Cache.setProperty(AUTOSUPERIMPOSE, + Boolean.valueOf(superimpose).toString()); setProgressBar(null, progressId); if (SiftsSettings.isMapWithSifts()) @@ -1069,7 +1114,7 @@ public class StructureChooser extends GStructureChooser } } } - if (seq.getPrimaryDBRefs().size() == 0) + if (seq.getPrimaryDBRefs().isEmpty()) { seqsWithoutSourceDBRef.add(seq); continue; @@ -1081,13 +1126,8 @@ public class StructureChooser extends GStructureChooser setProgressBar(MessageManager.formatMessage( "status.fetching_dbrefs_for_sequences_without_valid_refs", y), progressId); - SequenceI[] seqWithoutSrcDBRef = new SequenceI[y]; - int x = 0; - for (SequenceI fSeq : seqsWithoutSourceDBRef) - { - seqWithoutSrcDBRef[x++] = fSeq; - } - + SequenceI[] seqWithoutSrcDBRef = seqsWithoutSourceDBRef + .toArray(new SequenceI[y]); DBRefFetcher dbRefFetcher = new DBRefFetcher(seqWithoutSrcDBRef); dbRefFetcher.fetchDBRefs(true); @@ -1099,19 +1139,19 @@ public class StructureChooser extends GStructureChooser setProgressBar(MessageManager.getString( "status.fetching_3d_structures_for_selected_entries"), progressId); - sViewer.viewStructures(pdbEntriesToView, sequences, alignPanel); + theViewer.viewStructures(pdbEntriesToView, sequences, alignPanel); } else { setProgressBar(MessageManager.formatMessage( "status.fetching_3d_structures_for", pdbEntriesToView[0].getId()),progressId); - sViewer.viewStructures(pdbEntriesToView[0], sequences, alignPanel); + theViewer.viewStructures(pdbEntriesToView[0], sequences, alignPanel); } setProgressBar(null, progressId); // remember the last viewer we used... - lastTargetedView = sViewer; - return sViewer; + lastTargetedView = theViewer; + return theViewer; } /** @@ -1119,7 +1159,7 @@ public class StructureChooser extends GStructureChooser * a unique sequence when more than one sequence selection is made. */ @Override - public void populateCmbAssociateSeqOptions( + protected void populateCmbAssociateSeqOptions( JComboBox cmb_assSeq, JLabel lbl_associateSeq) { @@ -1144,17 +1184,12 @@ public class StructureChooser extends GStructureChooser } } - public boolean isStructuresDiscovered() + protected boolean isStructuresDiscovered() { return discoveredStructuresSet != null && !discoveredStructuresSet.isEmpty(); } - public Collection getDiscoveredStructuresSet() - { - return discoveredStructuresSet; - } - @Override protected void txt_search_ActionPerformed() { @@ -1204,7 +1239,7 @@ public class StructureChooser extends GStructureChooser } @Override - public void tabRefresh() + protected void tabRefresh() { if (selectedSequences != null) { diff --git a/src/jalview/gui/StructureViewer.java b/src/jalview/gui/StructureViewer.java index 19e0a22..35a4bc6 100644 --- a/src/jalview/gui/StructureViewer.java +++ b/src/jalview/gui/StructureViewer.java @@ -287,9 +287,10 @@ public class StructureViewer if (sview != null) { sview.setAlignAddedStructures(superposeAdded); - if (!sview.addAlreadyLoadedFile(seqsForPdb, null, ap, pdb.getId())) + String pdbId = pdb.getId(); + if (!sview.addAlreadyLoadedFile(seqsForPdb, null, ap, pdbId)) { - sview.addToExistingViewer(pdb, seqsForPdb, null, ap, pdb.getId()); + sview.addToExistingViewer(pdb, seqsForPdb, null, ap, pdbId); } sview.updateTitleAndMenus(); return sview; diff --git a/src/jalview/gui/StructureViewerBase.java b/src/jalview/gui/StructureViewerBase.java index 20f5c1f..840dbae 100644 --- a/src/jalview/gui/StructureViewerBase.java +++ b/src/jalview/gui/StructureViewerBase.java @@ -410,35 +410,23 @@ public abstract class StructureViewerBase extends GStructureViewer return Desktop.instance.getStructureViewers(alp, this.getClass()); } - - /** - * Check for any existing views involving this alignment and give user the - * option to add and align this molecule to one of them - * - * @param pdbentry - * @param seq - * @param chains - * @param apanel - * @param pdbId - * @return true if user adds to a view, or cancels entirely, else false - */ @Override - public boolean addToExistingViewer(PDBEntry pdbentry, SequenceI[] seq, + public void addToExistingViewer(PDBEntry pdbentry, SequenceI[] seq, String[] chains, final AlignmentViewPanel apanel, String pdbId) { /* * JAL-1742 exclude view with this structure already mapped (don't offer - * to align chain B to chain A of the same structure) + * to align chain B to chain A of the same structure); code may defend + * against this possibility before we reach here */ if (hasPdbId(pdbId)) { - return false; + return; } - AlignmentPanel ap = (AlignmentPanel) apanel; // Implementation error if this + AlignmentPanel alignPanel = (AlignmentPanel) apanel; // Implementation error if this // cast fails - useAlignmentPanelForSuperposition(ap); - addStructure(pdbentry, seq, chains, ap.alignFrame); - return true; + useAlignmentPanelForSuperposition(alignPanel); + addStructure(pdbentry, seq, chains, alignPanel.alignFrame); } /** @@ -502,33 +490,20 @@ public abstract class StructureViewerBase extends GStructureViewer } } - /** - * Check if the PDB file is already loaded, if so offer to add it to the - * existing viewer - * - * @param seq - * @param chains - * @param apanel - * @param pdbId - * @return true if the user chooses to add to a viewer, or to cancel entirely - */ @Override public boolean addAlreadyLoadedFile(SequenceI[] seq, String[] chains, final AlignmentViewPanel apanel, String pdbId) { - boolean finished = false; String alreadyMapped = apanel.getStructureSelectionManager() .alreadyMappedToFile(pdbId); - if (alreadyMapped != null) + if (alreadyMapped == null) { - /* - * the PDB file is already loaded - */ - addSequenceMappingsToStructure(seq, chains, apanel, alreadyMapped); - finished = true; + return false; } - return finished; + + addSequenceMappingsToStructure(seq, chains, apanel, alreadyMapped); + return true; } void setChainMenuItems(List chainNames) @@ -808,11 +783,11 @@ public abstract class StructureViewerBase extends GStructureViewer int[] alm = new int[_alignwith.size()]; int a = 0; - for (AlignmentPanel ap : _alignwith) + for (AlignmentPanel alignPanel : _alignwith) { - als[a] = ap.av.getAlignment(); + als[a] = alignPanel.av.getAlignment(); alm[a] = -1; - alc[a++] = ap.av.getAlignment().getHiddenColumns(); + alc[a++] = alignPanel.av.getAlignment().getHiddenColumns(); } reply = getBinding().superposeStructures(als, alm, alc); if (reply != null) @@ -824,9 +799,9 @@ public abstract class StructureViewerBase extends GStructureViewer } catch (Exception e) { StringBuffer sp = new StringBuffer(); - for (AlignmentPanel ap : _alignwith) + for (AlignmentPanel alignPanel : _alignwith) { - sp.append("'" + ap.alignFrame.getTitle() + "' "); + sp.append("'" + alignPanel.alignFrame.getTitle() + "' "); } Cache.log.info("Couldn't align structures with the " + sp.toString() + "associated alignment panels.", e); @@ -890,9 +865,9 @@ public abstract class StructureViewerBase extends GStructureViewer } } // Set the colour using the current view for the associated alignframe - for (AlignmentPanel ap : _colourwith) + for (AlignmentPanel alignPanel : _colourwith) { - binding.colourBySequence(ap); + binding.colourBySequence(alignPanel); } seqColoursApplied = true; } diff --git a/src/jalview/jbgui/GStructureChooser.java b/src/jalview/jbgui/GStructureChooser.java index 523ab7e..a30be66 100644 --- a/src/jalview/jbgui/GStructureChooser.java +++ b/src/jalview/jbgui/GStructureChooser.java @@ -37,6 +37,7 @@ import java.awt.CardLayout; import java.awt.Component; import java.awt.Dimension; import java.awt.FlowLayout; +import java.awt.Font; import java.awt.GridLayout; import java.awt.event.ActionEvent; import java.awt.event.ItemEvent; @@ -83,12 +84,23 @@ import net.miginfocom.swing.MigLayout; public abstract class GStructureChooser extends JPanel implements ItemListener { + private static final Font VERDANA_12 = new Font("Verdana", 0, 12); + + protected static final String VIEWS_FILTER = "VIEWS_FILTER"; + + protected static final String VIEWS_FROM_FILE = "VIEWS_FROM_FILE"; + + protected static final String VIEWS_ENTER_ID = "VIEWS_ENTER_ID"; + + /* + * 'cached' structure view + */ + protected static final String VIEWS_LOCAL_PDB = "VIEWS_LOCAL_PDB"; + protected JPanel statusPanel = new JPanel(); public JLabel statusBar = new JLabel(); - private JPanel pnl_actionsAndStatus = new JPanel(new BorderLayout()); - protected String frameTitle = MessageManager .getString("label.structure_chooser"); @@ -100,15 +112,9 @@ public abstract class GStructureChooser extends JPanel protected StringBuilder errorWarning = new StringBuilder(); - protected JLabel lbl_result = new JLabel( - MessageManager.getString("label.select")); - - protected JButton btn_view = new JButton(); - - protected JButton btn_newview = new JButton( - MessageManager.getString("label.new_view")); + protected JButton btn_add; - protected JButton btn_cancel = new JButton(); + protected JButton btn_newView; protected JButton btn_pdbFromFile = new JButton(); @@ -117,30 +123,11 @@ public abstract class GStructureChooser extends JPanel protected JTextField txt_search = new JTextField(14); - private JPanel pnl_actions = new JPanel(new MigLayout()); - - private JPanel pnl_main = new JPanel(); - - private JPanel pnl_idInput = new JPanel(new FlowLayout()); - - private JPanel pnl_fileChooser = new JPanel(new FlowLayout()); - - private JPanel pnl_idInputBL = new JPanel(new BorderLayout()); - - private JPanel pnl_fileChooserBL = new JPanel(new BorderLayout()); - - private JPanel pnl_locPDB = new JPanel(new BorderLayout()); - protected JPanel pnl_switchableViews = new JPanel(new CardLayout()); protected CardLayout layout_switchableViews = (CardLayout) (pnl_switchableViews .getLayout()); - private BorderLayout mainLayout = new BorderLayout(); - - protected JCheckBox chk_rememberSettings = new JCheckBox( - MessageManager.getString("label.dont_ask_me_again")); - protected JCheckBox chk_invertFilter = new JCheckBox( MessageManager.getString("label.invert")); @@ -156,35 +143,20 @@ public abstract class GStructureChooser extends JPanel protected ImageIcon warningImage = new ImageIcon( getClass().getResource("/images/warning.gif")); - protected JLabel lbl_warning = new JLabel(warningImage); - protected JLabel lbl_loading = new JLabel(loadingImage); protected JLabel lbl_pdbManualFetchStatus = new JLabel(errorImage); protected JLabel lbl_fromFileStatus = new JLabel(errorImage); - protected AssciateSeqPanel idInputAssSeqPanel = new AssciateSeqPanel(); + protected AssociateSeqPanel idInputAssSeqPanel = new AssociateSeqPanel(); - protected AssciateSeqPanel fileChooserAssSeqPanel = new AssciateSeqPanel(); + protected AssociateSeqPanel fileChooserAssSeqPanel = new AssociateSeqPanel(); - protected static final String VIEWS_FILTER = "VIEWS_FILTER"; - - protected static final String VIEWS_FROM_FILE = "VIEWS_FROM_FILE"; - - protected static final String VIEWS_ENTER_ID = "VIEWS_ENTER_ID"; - - protected JComboBox targetView = new JComboBox(); - - /** - * 'cached' structure view - */ - protected static final String VIEWS_LOCAL_PDB = "VIEWS_LOCAL_PDB"; + protected JComboBox targetView = new JComboBox<>(); protected JTable tbl_local_pdb = new JTable(); - protected JScrollPane scrl_localPDB = new JScrollPane(tbl_local_pdb); - protected JTabbedPane pnl_filter = new JTabbedPane(); protected FTSDataColumnPreferences pdbDocFieldPrefs = new FTSDataColumnPreferences( @@ -273,8 +245,6 @@ public abstract class GStructureChooser extends JPanel } }; - protected JScrollPane scrl_foundStructures = new JScrollPane(tbl_summary); - public GStructureChooser() { try @@ -330,9 +300,9 @@ public abstract class GStructureChooser extends JPanel mainFrame.dispose(); break; case KeyEvent.VK_ENTER: // enter key - if (btn_view.isEnabled()) + if (btn_add.isEnabled()) { - view_ActionPerformed(); + add_ActionPerformed(); } break; case KeyEvent.VK_TAB: // tab key @@ -342,7 +312,7 @@ public abstract class GStructureChooser extends JPanel } else { - btn_view.requestFocus(); + btn_add.requestFocus(); } evt.consume(); break; @@ -351,6 +321,30 @@ public abstract class GStructureChooser extends JPanel } } }); + + JButton btn_cancel = new JButton( + MessageManager.getString("action.cancel")); + btn_cancel.setFont(VERDANA_12); + btn_cancel.addActionListener(new java.awt.event.ActionListener() + { + @Override + public void actionPerformed(ActionEvent e) + { + closeAction(pnl_filter.getHeight()); + } + }); + btn_cancel.addKeyListener(new KeyAdapter() + { + @Override + public void keyPressed(KeyEvent evt) + { + if (evt.getKeyCode() == KeyEvent.VK_ENTER) + { + closeAction(pnl_filter.getHeight()); + } + } + }); + tbl_local_pdb.setAutoCreateRowSorter(true); tbl_local_pdb.getTableHeader().setReorderingAllowed(false); tbl_local_pdb.addMouseListener(new MouseAdapter() @@ -379,9 +373,9 @@ public abstract class GStructureChooser extends JPanel mainFrame.dispose(); break; case KeyEvent.VK_ENTER: // enter key - if (btn_view.isEnabled()) + if (btn_add.isEnabled()) { - view_ActionPerformed(); + add_ActionPerformed(); } break; case KeyEvent.VK_TAB: // tab key @@ -391,9 +385,9 @@ public abstract class GStructureChooser extends JPanel } else { - if (btn_view.isEnabled()) + if (btn_add.isEnabled()) { - btn_view.requestFocus(); + btn_add.requestFocus(); } else { @@ -407,73 +401,52 @@ public abstract class GStructureChooser extends JPanel } } }); - btn_newview.setFont(new java.awt.Font("Verdana", 0, 12)); - btn_newview.setText(MessageManager.getString("action.new_view")); - btn_newview.addActionListener(new java.awt.event.ActionListener() - { - @Override - public void actionPerformed(ActionEvent e) - { - newview_ActionPerformed(); - } - }); - btn_newview.addKeyListener(new KeyAdapter() - { - @Override - public void keyPressed(KeyEvent evt) - { - if (evt.getKeyCode() == KeyEvent.VK_ENTER) - { - newview_ActionPerformed(); - } - } - }); - btn_view.setFont(new java.awt.Font("Verdana", 0, 12)); - btn_view.setText(MessageManager.getString("action.add")); - btn_view.addActionListener(new java.awt.event.ActionListener() + btn_newView = new JButton(MessageManager.getString("action.new_view")); + btn_newView.setFont(VERDANA_12); + btn_newView.addActionListener(new java.awt.event.ActionListener() { @Override public void actionPerformed(ActionEvent e) { - view_ActionPerformed(); + newView_ActionPerformed(); } }); - btn_view.addKeyListener(new KeyAdapter() + btn_newView.addKeyListener(new KeyAdapter() { @Override public void keyPressed(KeyEvent evt) { if (evt.getKeyCode() == KeyEvent.VK_ENTER) { - view_ActionPerformed(); + newView_ActionPerformed(); } } }); - btn_cancel.setFont(new java.awt.Font("Verdana", 0, 12)); - btn_cancel.setText(MessageManager.getString("action.cancel")); - btn_cancel.addActionListener(new java.awt.event.ActionListener() + btn_add = new JButton(MessageManager.getString("action.add")); + btn_add.setFont(VERDANA_12); + btn_add.addActionListener(new java.awt.event.ActionListener() { @Override public void actionPerformed(ActionEvent e) { - closeAction(pnl_filter.getHeight()); + add_ActionPerformed(); } }); - btn_cancel.addKeyListener(new KeyAdapter() + btn_add.addKeyListener(new KeyAdapter() { @Override public void keyPressed(KeyEvent evt) { if (evt.getKeyCode() == KeyEvent.VK_ENTER) { - closeAction(pnl_filter.getHeight()); + add_ActionPerformed(); } } }); - btn_pdbFromFile.setFont(new java.awt.Font("Verdana", 0, 12)); + btn_pdbFromFile.setFont(VERDANA_12); String btn_title = MessageManager.getString("label.select_pdb_file"); btn_pdbFromFile.setText(btn_title + " "); btn_pdbFromFile.addActionListener(new java.awt.event.ActionListener() @@ -496,20 +469,17 @@ public abstract class GStructureChooser extends JPanel } }); + JScrollPane scrl_foundStructures = new JScrollPane(tbl_summary); scrl_foundStructures.setPreferredSize(new Dimension(width, height)); + JScrollPane scrl_localPDB = new JScrollPane(tbl_local_pdb); scrl_localPDB.setPreferredSize(new Dimension(width, height)); scrl_localPDB.setHorizontalScrollBarPolicy( JScrollPane.HORIZONTAL_SCROLLBAR_NEVER); - cmb_filterOption.setFont(new java.awt.Font("Verdana", 0, 12)); - chk_invertFilter.setFont(new java.awt.Font("Verdana", 0, 12)); - chk_rememberSettings.setFont(new java.awt.Font("Verdana", 0, 12)); - chk_rememberSettings.setVisible(false); + chk_invertFilter.setFont(VERDANA_12); txt_search.setToolTipText(JvSwingUtils.wrapTooltip(true, MessageManager.getString("label.enter_pdb_id_tip"))); - cmb_filterOption.setToolTipText( - MessageManager.getString("info.select_filter_option")); txt_search.getDocument().addDocumentListener(new DocumentListener() { @Override @@ -531,8 +501,10 @@ public abstract class GStructureChooser extends JPanel } }); + cmb_filterOption.setFont(VERDANA_12); + cmb_filterOption.setToolTipText( + MessageManager.getString("info.select_filter_option")); cmb_filterOption.addItemListener(this); - // add CustomComboSeparatorsRenderer to filter option combo-box cmb_filterOption.setRenderer(new CustomComboSeparatorsRenderer( (ListCellRenderer) cmb_filterOption.getRenderer()) @@ -548,27 +520,32 @@ public abstract class GStructureChooser extends JPanel chk_invertFilter.addItemListener(this); targetView.setVisible(false); - pnl_actions.add(targetView, "left"); - pnl_actions.add(btn_view, "wrap"); - pnl_actions.add(chk_superpose, "left"); - pnl_actions.add(btn_newview); - pnl_actions.add(btn_cancel, "right"); - // pnl_actions.add(chk_rememberSettings); + JPanel actionsPanel = new JPanel(new MigLayout()); + actionsPanel.add(targetView, "left"); + actionsPanel.add(btn_add, "wrap"); + actionsPanel.add(chk_superpose, "left"); + actionsPanel.add(btn_newView); + actionsPanel.add(btn_cancel, "right"); - // pnl_filter.add(lbl_result); + JPanel pnl_main = new JPanel(); pnl_main.add(cmb_filterOption); pnl_main.add(lbl_loading); pnl_main.add(chk_invertFilter); lbl_loading.setVisible(false); + JPanel pnl_fileChooser = new JPanel(new FlowLayout()); pnl_fileChooser.add(btn_pdbFromFile); pnl_fileChooser.add(lbl_fromFileStatus); + JPanel pnl_fileChooserBL = new JPanel(new BorderLayout()); pnl_fileChooserBL.add(fileChooserAssSeqPanel, BorderLayout.NORTH); pnl_fileChooserBL.add(pnl_fileChooser, BorderLayout.CENTER); + JPanel pnl_idInput = new JPanel(new FlowLayout()); pnl_idInput.add(txt_search); pnl_idInput.add(lbl_pdbManualFetchStatus); + + JPanel pnl_idInputBL = new JPanel(new BorderLayout()); pnl_idInputBL.add(idInputAssSeqPanel, BorderLayout.NORTH); pnl_idInputBL.add(pnl_idInput, BorderLayout.CENTER); @@ -584,15 +561,15 @@ public abstract class GStructureChooser extends JPanel JTabbedPane sourceTabbedPane = (JTabbedPane) changeEvent .getSource(); int index = sourceTabbedPane.getSelectedIndex(); - btn_view.setVisible(targetView.isVisible()); - btn_newview.setVisible(true); + btn_add.setVisible(targetView.isVisible()); + btn_newView.setVisible(true); btn_cancel.setVisible(true); if (sourceTabbedPane.getTitleAt(index).equals(configureCols)) { - btn_view.setEnabled(false); + btn_add.setEnabled(false); btn_cancel.setEnabled(false); - btn_view.setVisible(false); - btn_newview.setEnabled(false); + btn_add.setVisible(false); + btn_newView.setEnabled(false); btn_cancel.setVisible(false); previousWantedFields = pdbDocFieldPrefs .getStructureSummaryFields() @@ -618,6 +595,7 @@ public abstract class GStructureChooser extends JPanel pnl_filter.add(foundStructureSummary, scrl_foundStructures); pnl_filter.add(configureCols, pdbDocFieldPrefs); + JPanel pnl_locPDB = new JPanel(new BorderLayout()); pnl_locPDB.add(scrl_localPDB); pnl_switchableViews.add(pnl_fileChooserBL, VIEWS_FROM_FILE); @@ -625,12 +603,14 @@ public abstract class GStructureChooser extends JPanel pnl_switchableViews.add(pnl_filter, VIEWS_FILTER); pnl_switchableViews.add(pnl_locPDB, VIEWS_LOCAL_PDB); - this.setLayout(mainLayout); + this.setLayout(new BorderLayout()); this.add(pnl_main, java.awt.BorderLayout.NORTH); this.add(pnl_switchableViews, java.awt.BorderLayout.CENTER); // this.add(pnl_actions, java.awt.BorderLayout.SOUTH); statusPanel.setLayout(new GridLayout()); - pnl_actionsAndStatus.add(pnl_actions, BorderLayout.CENTER); + + JPanel pnl_actionsAndStatus = new JPanel(new BorderLayout()); + pnl_actionsAndStatus.add(actionsPanel, BorderLayout.CENTER); pnl_actionsAndStatus.add(statusPanel, BorderLayout.SOUTH); statusPanel.add(statusBar, null); this.add(pnl_actionsAndStatus, java.awt.BorderLayout.SOUTH); @@ -841,13 +821,13 @@ public abstract class GStructureChooser extends JPanel * @author tcnofoegbu * */ - public class AssciateSeqPanel extends JPanel implements ItemListener + public class AssociateSeqPanel extends JPanel implements ItemListener { private JComboBox cmb_assSeq = new JComboBox<>(); private JLabel lbl_associateSeq = new JLabel(); - public AssciateSeqPanel() + public AssociateSeqPanel() { this.setLayout(new FlowLayout()); this.add(cmb_assSeq); @@ -941,21 +921,21 @@ public abstract class GStructureChooser extends JPanel protected abstract void stateChanged(ItemEvent e); - protected abstract void view_ActionPerformed(); + protected abstract void add_ActionPerformed(); - protected abstract void newview_ActionPerformed(); + protected abstract void newView_ActionPerformed(); protected abstract void pdbFromFile_actionPerformed(); protected abstract void txt_search_ActionPerformed(); - public abstract void populateCmbAssociateSeqOptions( + protected abstract void populateCmbAssociateSeqOptions( JComboBox cmb_assSeq, JLabel lbl_associateSeq); - public abstract void cmbAssSeqStateChanged(); + protected abstract void cmbAssSeqStateChanged(); - public abstract void tabRefresh(); + protected abstract void tabRefresh(); - public abstract void validateSelections(); + protected abstract void validateSelections(); } \ No newline at end of file diff --git a/src/jalview/structure/StructureSelectionManager.java b/src/jalview/structure/StructureSelectionManager.java index 10fe836..cdb4e0a 100644 --- a/src/jalview/structure/StructureSelectionManager.java +++ b/src/jalview/structure/StructureSelectionManager.java @@ -285,7 +285,8 @@ public class StructureSelectionManager } /** - * Returns the file name for a mapped PDB id (or null if not mapped). + * Returns the filename the PDB id is already mapped to if known, or null if + * it is not mapped * * @param pdbid * @return @@ -294,7 +295,7 @@ public class StructureSelectionManager { for (StructureMapping sm : mappings) { - if (sm.getPdbId().equals(pdbid)) + if (sm.getPdbId().equalsIgnoreCase(pdbid)) { return sm.pdbfile; } diff --git a/test/jalview/gui/StructureChooserTest.java b/test/jalview/gui/StructureChooserTest.java index 91fe602..f69e6b5 100644 --- a/test/jalview/gui/StructureChooserTest.java +++ b/test/jalview/gui/StructureChooserTest.java @@ -21,6 +21,7 @@ package jalview.gui; import static org.testng.AssertJUnit.assertEquals; +import static org.testng.AssertJUnit.assertNotNull; import static org.testng.AssertJUnit.assertTrue; import jalview.datamodel.DBRefEntry; @@ -28,8 +29,10 @@ import jalview.datamodel.DBRefSource; import jalview.datamodel.PDBEntry; import jalview.datamodel.Sequence; import jalview.datamodel.SequenceI; +import jalview.fts.api.FTSData; import jalview.jbgui.GStructureChooser.FilterOption; +import java.util.Collection; import java.util.Vector; import org.testng.annotations.AfterMethod; @@ -37,6 +40,8 @@ import org.testng.annotations.BeforeClass; import org.testng.annotations.BeforeMethod; import org.testng.annotations.Test; +import junit.extensions.PA; + public class StructureChooserTest { @@ -142,8 +147,10 @@ public class StructureChooserTest SequenceI[] selectedSeqs = new SequenceI[] { seq }; StructureChooser sc = new StructureChooser(selectedSeqs, seq, null); sc.fetchStructuresMetaData(); - assertTrue(sc.getDiscoveredStructuresSet() != null); - assertTrue(sc.getDiscoveredStructuresSet().size() > 0); + Collection ss = (Collection) PA.getValue(sc, + "discoveredStructuresSet"); + assertNotNull(ss); + assertTrue(ss.size() > 0); } -- 1.7.10.2