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

convert bibtexkeypatterndlg to javafx as well #4277

Merged
merged 2 commits into from
Aug 22, 2018

Conversation

Siedlerchr
Copy link
Member

@Siedlerchr Siedlerchr commented Aug 20, 2018

Followup from #4253 in the Bibtex key pattern panel there was an exception thrown if you had defined custom entry types or used biblatex mode.

@Super-Tang In your PR you used a fixed size for the number of labels and buttons. However, as it is possible in JabRef to customize the entry types, e.g. add new ones, the size is no longer fixed. Therefore if the number of entry types was > 19 the array threw an exception.
I now used the size of the entry type list to setup the array size and wrapped it in a Scroll Pane

Edit// I set colspan to remaining, so it uses all space
grafik


  • 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?)

@Siedlerchr Siedlerchr requested a review from tobiasdiez August 20, 2018 10:39
@Siedlerchr Siedlerchr added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Aug 20, 2018
@Siedlerchr
Copy link
Member Author

I decided to quickly convert the Bibtex Key Pattern Dialog as well to javafx., it is using the same panel from the pref tabs

@Super-Tang
Copy link
Contributor

Sorry for not thinking so much. :( @Siedlerchr

@Siedlerchr
Copy link
Member Author

@Super-Tang No problem! Now you know it for the future and I bet you won't come in such a situation again ;)

@tobiasdiez
Copy link
Member

I implemented a similar fix in #4280, which goes even a step further and completely removes the textbox array. Sorry for not being quicker with my PR and the double effort. So that we don't run in a merge hell, I would propose you remove your changes to this key pattern pane class. The code looks good otherwise.

BibtexKeyPatternDialog bibtexKeyPatternDialog = new BibtexKeyPatternDialog(frame.getCurrentBasePanel());
bibtexKeyPatternDialog.setLocationRelativeTo(null);
bibtexKeyPatternDialog.setVisible(true);
BibtexKeyPatternDialog dlg = new BibtexKeyPatternDialog(frame.getCurrentBasePanel());
Copy link
Member

Choose a reason for hiding this comment

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

Please no abbreviations

if (button == ButtonType.APPLY) {
metaData.setCiteKeyPattern(bibtexKeyPatternPanel.getKeyPatternAsDatabaseBibtexKeyPattern());
panel.markNonUndoableBaseChanged();
} else {
Copy link
Member

Choose a reason for hiding this comment

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

else branch is not really needed

@@ -20,7 +20,7 @@ protected BaseDialog() {
});

setDialogIcon(IconTheme.getJabRefImageFX());

setResizable(true);
Copy link
Member

Choose a reason for hiding this comment

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

It's a good idea to set this globally. Can you please remove the other explicit setResizable calls that are now no longer needed.

Copy link
Member Author

Choose a reason for hiding this comment

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

I removed it everywhere except in the prefs dialog to not create a conflict with your other PR

@Siedlerchr Siedlerchr force-pushed the fixbibtexkeypatternprefs branch from d4c58e4 to 8a1d935 Compare August 22, 2018 08:33
@Siedlerchr Siedlerchr changed the title Fix exception with biblatex mode and wrap in scroll pane convert bibtexkeypatterndlg to javafx as well Aug 22, 2018
@tobiasdiez tobiasdiez merged commit 2dbb05e into master Aug 22, 2018
@tobiasdiez tobiasdiez deleted the fixbibtexkeypatternprefs branch August 22, 2018 15:18
Siedlerchr added a commit that referenced this pull request Aug 24, 2018
* upstream/master: (129 commits)
  Fix Export to clipbaord action (#4286)
  Add minimal height for entry editor (#4283)
  Fix exceptions when trying to change settings (#4284)
  Added a comment for the purpose of the class (#4282)
  Fix NPE when importing from CLI (#4281)
  Fix display of key patterns
  convert bibtexkeypatterndlg to javafx as well (#4277)
  Style preference sections using css
  Fix a few style issues
  Changed the behavior of the field formatting dialog (#4275)
  Fix checkbox display
  Remove small font size
  Style side pane in preferences
  Wrap preference tabs in scrollpane
  Fix NPE with bibdatabasecontext in preview style dialog
  update xmlunit and log4j jul and checkstyle
  Update mysql driver to 8.0.12
  Rework preference dialog main class
  Move preferences stuff to gui.preferences
  Implement drag and drop for maintable (#3765)
  ...

# Conflicts:
#	src/main/java/org/jabref/gui/shared/ConnectToSharedDatabaseDialog.java
Siedlerchr added a commit that referenced this pull request Aug 29, 2018
* upstream/master: (468 commits)
  Fix error prone warnings (#4290)
  Fix freeze on import into current library (#4299)
  Upgrade gradle to 4.10 update xmlunit and postgres
  update xmlunit-matchers from 2.6.0 -> 2.6.1
  update gradle build-scan from 1.15.2 -> 1.16
  checkstyle
  execute changes only if disk db present
  Fix Export to clipbaord action (#4286)
  fix npe in Merge entries dialog
  Add minimal height for entry editor (#4283)
  Fix exceptions when trying to change settings (#4284)
  Added a comment for the purpose of the class (#4282)
  Fix NPE when importing from CLI (#4281)
  Fix display of key patterns
  convert bibtexkeypatterndlg to javafx as well (#4277)
  Style preference sections using css
  Fix a few style issues
  Changed the behavior of the field formatting dialog (#4275)
  Fix checkbox display
  Remove small font size
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants