[FIX] Copy action #293
[FIX] Copy action #293
Conversation
shubhamWaghmare-sap
left a comment
shubhamWaghmare-sap
left a comment
There was a problem hiding this comment.
Minor refactoring changes.
| manager.add(AbapGitView.this.actionCopy); | ||
|
|
||
| // Create the sub-menu for Copy | ||
| MenuManager copySubMenu = new MenuManager("Copy"); |
There was a problem hiding this comment.
Externalize the string.
| this.actionCopyCell = new Action("Copy Cell") { | ||
| @Override | ||
| public void run() { | ||
| copyCell(); | ||
| } | ||
| }; | ||
|
|
||
| this.actionCopyRow = new Action("Copy Row") { | ||
| @Override | ||
| public void run() { | ||
| copyRow(); | ||
| } | ||
| }; | ||
|
|
There was a problem hiding this comment.
Externalize both strings.
| final Clipboard clipboard = new Clipboard(this.viewer.getControl().getDisplay()); | ||
| String textToCopy = null; | ||
|
|
||
| TableItem[] selection = this.viewer.getTable().getSelection(); | ||
| if (selection.length > 0 && this.focusedColumnIndex != -1) { | ||
| textToCopy = selection[0].getText(this.focusedColumnIndex); | ||
| } | ||
|
|
||
| if (textToCopy != null && !textToCopy.isEmpty()) { | ||
| clipboard.setContents(new String[] { textToCopy }, new TextTransfer[] { TextTransfer.getInstance() }); | ||
| } | ||
| clipboard.dispose(); |
There was a problem hiding this comment.
Copying to clipboard is common in both the copyCell & copyRow methods. Extract the logic as a separate method and use it in the copy methods.
private void copyToClipboard(String textToCopy) {
if (textToCopy == null || textToCopy.isEmpty()) return;
final Clipboard clipboard = new Clipboard(this.viewer.getControl().getDisplay());
clipboard.setContents(new String[] { textToCopy },
new TextTransfer[] { TextTransfer.getInstance() });
clipboard.dispose();
}
}
| return; // Found the column | ||
| } | ||
| } | ||
| AbapGitView.this.focusedColumnIndex = -1; |
There was a problem hiding this comment.
Low prio: Can extract -1 as a new constant. Say NO_COLUMN_SELECTED.
There was a problem hiding this comment.
Can extract the complete logic to find the selected column into a separate method. Easier to understand what's happening via the method name.
| table.addMouseListener(new MouseAdapter() { | ||
| @Override | ||
| public void mouseDown(MouseEvent e) { | ||
| Point pt = new Point(e.x, e.y); |
There was a problem hiding this comment.
Use full variable name wherever possible. i.e. use point instead of pt.
2 participants
Changes Made
Previous flow:
when clicked on copyAction whole row was copied and copied text have to be truncatedNew flow: