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

Refactor SaveOrderConfigPanel #7935

Merged
merged 23 commits into from
Aug 5, 2021
Merged

Refactor SaveOrderConfigPanel #7935

merged 23 commits into from
Aug 5, 2021

Conversation

calixtus
Copy link
Member

@calixtus calixtus commented Jul 23, 2021

A messy job, that was on my ToDo list for some time now.
I gave the SaveOrderConfigPanel an overhaul to make it a little bit more generic and to be able to work with more than three SortCriteria.

Before:
grafik

After:
grafik

One Problem remaining is that the preferences do only allow three SortCriteria for Export at all with the current JabRefPreferences implementation. Changing that would mean I would have to add PreferenceMigrations OR i just allow three SortCriteria for now in the Export prefs tab.

Yet some very little issues left, i hope i can fix them the next days.

  • Change in CHANGELOG.md described in a way that is understandable for the average user (if applicable)
  • Tests created for changes (if applicable)
  • Manually tested changed features in running JabRef (always required)
  • Screenshots added in PR description (for UI changes)
  • Checked documentation: Is the information available and up to date? If not created an issue at https://github.com/JabRef/user-documentation/issues or, even better, submitted a pull request to the documentation repository.

@Siedlerchr
Copy link
Member

Good! While you are at it, can you please take a look at https://discourse.jabref.org/t/sort-order-for-export/2598/12

@calixtus
Copy link
Member Author

calixtus commented Jul 24, 2021

Hmm... I think I unintentionally stumbled over a bug in the tests.
The 'expected' value of in the test bib file is
image
while the test states that
image

EDIT: No, understood it wrong.

@calixtus
Copy link
Member Author

Good! While you are at it, can you please take a look at https://discourse.jabref.org/t/sort-order-for-export/2598/12

I noticed, that when i change the number of criterias and save it, JabRef complains about the file being saved by another program. Could have something to do with it?

I can take a deeper look into this, but I'm somewhat careful, because I don't want this PR to explode beyond dealing with the visual representation. Let's see where this is going.

@calixtus
Copy link
Member Author

calixtus commented Aug 2, 2021

DevCall decision:

@calixtus calixtus marked this pull request as ready for review August 2, 2021 21:57
@calixtus calixtus added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Aug 2, 2021
@koppor koppor changed the title [WIP] Refactor SaveOrderConfigPanel Refactor SaveOrderConfigPanel Aug 2, 2021
@koppor
Copy link
Member

koppor commented Aug 2, 2021

Fixes JabRef#510

@Siedlerchr Siedlerchr merged commit 3a44c69 into main Aug 5, 2021
@Siedlerchr Siedlerchr deleted the refactor-saveorder branch August 5, 2021 13:57
@calixtus calixtus removed the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Aug 12, 2021
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.

3 participants