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 confirmation for 'Copy to' with cross-reference status #12493

Merged
merged 4 commits into from
Feb 15, 2025

Conversation

MhammedAhmmed
Copy link
Contributor

@MhammedAhmmed MhammedAhmmed commented Feb 12, 2025

fixes #12486

Add confirmation message after using "Copy to" option, also show whether the cross-reference entry was copied or not.

Mandatory checks

  • I own the copyright of the code submitted and I licence it under the MIT license
  • Change in CHANGELOG.md described in a way that is understandable for the average user (if change is visible to the user)
  • Tests created for changes (if applicable)
  • Manually tested changed features in running JabRef (always required)
  • Screenshots added in PR description (for UI changes)
  • Checked developer's documentation: Is the information available and up to date? If not, I outlined it in this pull request.
  • Checked documentation: Is the information available and up to date? If not, I created an issue at https://github.com/JabRef/user-documentation/issues or, even better, I submitted a pull request to the documentation repository.

Copy to with cross-reference included
bandicam 2025-02-12 15-19-03-300 - frame at 0m18s
Copy to with cross-reference excluded
bandicam 2025-02-12 15-19-03-300 - frame at 0m8s

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

JUnit tests are failing. In the area "Some checks were not successful", locate "Tests / Unit tests (pull_request)" and click on "Details". This brings you to the test output.

You can then run these tests in IntelliJ to reproduce the failing tests locally. We offer a quick test running howto in the section Final build system checks in our setup guide.

@@ -61,8 +61,10 @@ public void execute() {

if (includeCrossReferences) {
copyEntriesWithCrossRef(selectedEntries, targetDatabaseContext);
dialogService.notify(Localization.lang("Entry copied successfully, including cross-reference."));
Copy link
Member

@subhramit subhramit Feb 12, 2025

Choose a reason for hiding this comment

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

Please change to "entries" and "references".
(Tip - Here is how I, as a newcomer, would gather context - selectedEntries above is a list, and the name is plural as well. The function name also hints at the same. So it cannot be a single entry. I would go one level further and ctrl + click on the function to see what it's doing).

} else {
copyEntriesWithoutCrossRef(selectedEntries, targetDatabaseContext);
dialogService.notify(Localization.lang("Entry copied successfully, no cross-reference included."));
Copy link
Member

Choose a reason for hiding this comment

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

Same as above

src/main/resources/l10n/JabRef_en.properties Outdated Show resolved Hide resolved
Comment on lines 2836 to 2837
Entry\ copied\ successfully,\ including\ cross-reference.=Entry copied successfully, including cross-reference.
Entry\ copied\ successfully,\ no\ cross-reference\ included.=Entry copied successfully, no cross-reference included.
Copy link
Member

Choose a reason for hiding this comment

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

Change accordingly. Also, change the second line to "...without cross-references."

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the feedback! I will make sure to read functions carefully next time..

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

JUnit tests are failing. In the area "Some checks were not successful", locate "Tests / Unit tests (pull_request)" and click on "Details". This brings you to the test output.

You can then run these tests in IntelliJ to reproduce the failing tests locally. We offer a quick test running howto in the section Final build system checks in our setup guide.

Siedlerchr
Siedlerchr previously approved these changes Feb 15, 2025
@Siedlerchr Siedlerchr added this pull request to the merge queue Feb 15, 2025
Merged via the queue into JabRef:main with commit 4ca6a74 Feb 15, 2025
24 checks passed
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.

Improve UX for "Copy to" option with success dialog
3 participants