Skip to content
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

Add option for direct attached file renaming #4887

Merged
merged 23 commits into from
Apr 17, 2019

Conversation

kaiquekk
Copy link
Contributor

Add option on the Linked File Editor context menu to directly rename an entry's attached file. The option opens a dialog with a textfield that allows to change the name or the path of the attached file.

filerename
filerenamedialog

This PR fixes #4844


  • Change in CHANGELOG.md described
  • Tests created for changes
  • Manually tested changed features in running JabRef
  • Screenshots added in PR description (for bigger UI changes)
  • Ensured that the git commit message is a good one
  • Check documentation status (Issue created for outdated help page at help.jabref.org?)

.load()
.setAsContent(this.getDialogPane());

this.getDialogPane().getButtonTypes().addAll(ButtonType.APPLY, ButtonType.CANCEL);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can put these directly in the fxml, see some other fxml files. But don't forget the ButtonType import declaration in the fxml

@Siedlerchr
Copy link
Member

Thanks for the contribution, for the dialog I think it would make more sense to actually only change the filename and not the whole path. This would be confusing And I would rename the link label to "New Filename"
https://docs.oracle.com/javase/8/docs/api/java/nio/file/Path.html#getFileName--

@kaiquekk
Copy link
Contributor Author

Thanks for the reply. We tried to do the proposed alterations. Now it doesn't change the whole path, just the filename.

renamedialog

Copy link
Member

@tobiasdiez tobiasdiez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your contribution.

@@ -355,6 +356,19 @@ public void edit() {
});
}

public void renameFile() {
LinkedFileRenameDialogView dialog = new LinkedFileRenameDialogView(this.linkedFile);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm really sorry for your work, but in my opinion a simple TextInputDialog should be sufficient (instead of a custom dialog):

public Optional<String> showInputDialogAndWait(String title, String content) {

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tobiasdiez A TextInputDialog does not have a browse button....

Path subPath = Paths.get(oldFile).subpath(0, nameCount - 1);
Optional<LinkedFile> editedFile = dialog.showAndWait();
editedFile.ifPresent(file -> {
String newFile = System.getProperty("file.separator") + subPath.toString() + System.getProperty("file.separator") + file.getLink();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can use the Path.resolveSibling method to construct the new path more easily.

kaiquekk and others added 17 commits April 16, 2019 14:02
Add RENAME action on Standard Actions and add an item for it on the
right click menu of the Base Panel.

Signed-off-by: kaiquekk <kaique.komata@gmail.com>
Signed-off-by: yurickyh <yurickyussuke@hotmail.com>
Signed-off-by: kaiquekk <kaique.komata@gmail.com>
Remove the Rename button from the main table and add it on the linked
files editor table.

Signed-off-by: kaiquekk <kaique.komata@gmail.com>
Signed-off-by: yurickyh <yurickyussuke@hotmail.com>
Signed-off-by: yurickyh <yurickyussuke@hotmail.com>
Signed-off-by: yurickyh <yurickyussuke@hotmail.com>
Signed-off-by: yurickyh <yurickyussuke@hotmail.com>
Signed-off-by: yurickyh <yurickyussuke@hotmail.com>
Signed-off-by: kaiquekk <kaique.komata@gmail.com>
Signed-off-by: kaiquekk <kaique.komata@gmail.com>
Change the attached file rename dialog to only alter the filename and not the
whole file path.

Signed-off-by: kaiquekk <kaique.komata@gmail.com>
Signed-off-by: kaiquekk <kaique.komata@gmail.com>
…ecreased.

Signed-off-by: yurickyh <yurickyussuke@hotmail.com>
…re converted to ButtonType.

Signed-off-by: yurickyh <yurickyussuke@hotmail.com>
Signed-off-by: kaiquekk <kaique.komata@gmail.com>
Signed-off-by: kaiquekk <kaique.komata@gmail.com>
@kaiquekk kaiquekk force-pushed the fix-for-issue-4844 branch from 3748c65 to 169af5c Compare April 16, 2019 17:16
Signed-off-by: kaiquekk <kaique.komata@gmail.com>
@kaiquekk
Copy link
Contributor Author

As the intention is to only rename the file and not change the link, we thought that it makes sense to only have the textfield on the dialog. So, we changed it to a simple TextInputDialog like @tobiasdiez requested.

filerenamedialog

Copy link
Member

@matthiasgeiger matthiasgeiger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the follow-up!

But now I have another request ;-) Could you please add the old file name as the default text in the TextInputDialog?

As this option is currently not available in our DialogService I would suggest to add another method Optional<String> showInputDialogWithDefaultAndWait(String title, String content, String defaultValue);

As this is directly support by Java FX's TextInputDialog the implementation of the method is super easy.

Another aspect: looking at the screenshots I think we should consider relabeling also the old "Rename file" option - because for me this is not really self-explanatory. What about "Rename file to defined pattern"? Or "Rename to pattern" to have it shorter?
@tobiasdiez, @Siedlerchr WDYT?

Best regards,
Matthias

@tobiasdiez
Copy link
Member

@kaiquekk Thanks for the follow-up!
@matthiasgeiger Good suggestion. Rename file to defined pattern is ok for me (although a bit lengthy).

Signed-off-by: yurickyh <yurickyussuke@hotmail.com>
…extfield the old filename.

Signed-off-by: yurickyh <yurickyussuke@hotmail.com>
Signed-off-by: yurickyh <yurickyussuke@hotmail.com>
…ndWait.

Signed-off-by: yurickyh <yurickyussuke@hotmail.com>
Signed-off-by: yurickyh <yurickyussuke@hotmail.com>
@yurickyh
Copy link
Contributor

@tobiasdiez @matthiasgeiger : Thanks for your reply. We implemented what you suggested, so the text field has the old filename as its default value. Also, the 'Rename file' option was changed to 'Rename file to defined pattern' as asked.

prtSc

Copy link
Member

@matthiasgeiger matthiasgeiger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perfect! Thank you very much!

Copy link
Member

@tobiasdiez tobiasdiez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks again

@tobiasdiez tobiasdiez merged commit e230a95 into JabRef:master Apr 17, 2019
Siedlerchr added a commit that referenced this pull request Apr 18, 2019
* upstream/master:
  Add option for direct attached file renaming (#4887)
  Fixed Closing of the Shared Database Login Dialog if the user enters wrong authentication details - #4857 (#4892)

# Conflicts:
#	src/main/resources/l10n/JabRef_en.properties
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Direct file renaming
5 participants