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

Observable Preferences J (Preview) and minor refactor #8370

Merged
merged 12 commits into from
Jan 3, 2022
Merged

Conversation

calixtus
Copy link
Member

@calixtus calixtus commented Dec 29, 2021

Follow up to #8336
Preview as a separate tab pref should now have effect without need to restart jabref.
Also moved the search box in the preview panel below the list.
grafik

  • 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, I created an issue at https://github.com/JabRef/user-documentation/issues or, even better, I submitted a pull request to the documentation repository.

@calixtus calixtus added entry-editor entry-preview preferences status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers type: code-quality Issues related to code or architecture decisions labels Dec 29, 2021
@Siedlerchr
Copy link
Member

I don't like the idea of adding the search box below the lists. That's not intuitive. A search bar at the top allows the users to quickly browse through the avaiable styles
The current status is also consistent with the journal abbrevation as well

@calixtus
Copy link
Member Author

calixtus commented Jan 2, 2022

Well, I did not like the search box over the full length of both lists, even though the search filter only applies to the available list.
How about a compromise: I'll put the search box above the available list, but only the available list?

@calixtus
Copy link
Member Author

calixtus commented Jan 2, 2022

grafik

@@ -2437,66 +2435,65 @@ private void purgeCustomExportFormats(int from) {

@Override
public PreviewPreferences getPreviewPreferences() {
if (this.previewPreferences == null) {
updatePreviewPreferences();
if (Objects.nonNull(previewPreferences)) {
Copy link
Member

Choose a reason for hiding this comment

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

Usage of Objects.nonNull is only useful in predicates/lambdas:

This method exists to be used as a Predicate, filter(Objects::nonNull)

Copy link
Member Author

Choose a reason for hiding this comment

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

We already used this method in multiple locations in JabRef and everywhere in the preference conversion complex. For consistency in the code I'd like to continue to use it. The api-note, that this method especially exists for use as a Predicate doesn't mean that it couldn't be used for something else too. 😅

@Siedlerchr
Copy link
Member

I've found some kind of problems with selecting another style.
grafik

Now select an entry and cycle to the next style with F9,
it stays at the Customized Preview style and the notification "Preview style changed" is triggered multiple times and the style does not get changed

@Siedlerchr
Copy link
Member

Further investigation showed: The issue occurs if the Customized Preview style is not the firs tone

# Conflicts:
#	src/main/java/org/jabref/gui/collab/EntryAddChangeViewModel.java
#	src/main/java/org/jabref/gui/entryeditor/EntryEditor.java
#	src/main/java/org/jabref/gui/entryeditor/FieldsEditorTab.java
#	src/main/java/org/jabref/gui/preferences/PreferencesDialogView.java
#	src/main/java/org/jabref/gui/preferences/preview/PreviewTab.java
#	src/main/java/org/jabref/gui/preview/PreviewPanel.java
#	src/main/java/org/jabref/gui/search/GlobalSearchResultDialog.java
Copy link
Member

@Siedlerchr Siedlerchr left a comment

Choose a reason for hiding this comment

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

Works for me

@Siedlerchr Siedlerchr merged commit c94bb1b into main Jan 3, 2022
@Siedlerchr Siedlerchr deleted the preview-prefs branch January 3, 2022 23:25
Siedlerchr added a commit that referenced this pull request Jan 4, 2022
* upstream/main:
  Observable Preferences J (Preview) and minor refactor (#8370)
  quickfix (#8383)
  Live reloading when switching themes (#7336)
  FIX NPE on Merge Dialog (#8380)
  Bump org.eclipse.jgit from 5.13.0.202109080827-r to 6.0.0.202111291000-r (#8378)
  Bump jackson-dataformat-yaml from 2.13.0 to 2.13.1 (#8379)
  Bump tika-core from 2.2.0 to 2.2.1 (#8377)
  Update turabian-fullnote-bibliography-no-ibid.csl
  Squashed 'buildres/csl/csl-locales/' changes from d5ee85de8e..c38205618f
  Squashed 'buildres/csl/csl-styles/' changes from 60bf7d5..f78c707
Siedlerchr added a commit that referenced this pull request Jan 4, 2022
* upstream/main:
  Observable Preferences J (Preview) and minor refactor (#8370)
  quickfix (#8383)
  Live reloading when switching themes (#7336)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
entry-editor entry-preview preferences status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers type: code-quality Issues related to code or architecture decisions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants