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.

Sign up for GitHub

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

Jump to bottom

Scrollbar invisible in Preferences -> BibTex Key Pattern (#4287) #4398

Merged
merged 7 commits into from
Oct 24, 2018
Merged

Scrollbar invisible in Preferences -> BibTex Key Pattern (#4287) #4398

merged 7 commits into from
Oct 24, 2018

Conversation

pavlapp
Copy link
Contributor

@pavlapp pavlapp commented Oct 22, 2018 •
edited
Loading

  • fix invisible scrollbar in preferences
  • divided gridpane to multiple columns

preferences

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

@pavlapp
Scrollbar invisible in Preferences -> BibTex Key Pattern (JabRef#4287)
2c1b56f 
* fix invisible scrollbar in preferences
* divided gridpane to multiple columns
Siedlerchr
Siedlerchr requested changes Oct 22, 2018
View reviewed changes
src/main/java/org/jabref/gui/bibtexkeypattern/BibtexKeyPatternPanel.java Outdated
gridPane.add(defaultPat, 2, rowIndex);
gridPane.add(button, 3, rowIndex);

Object[] entryTypes = EntryTypes.getAllValues(mode).toArray();
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 also iterate over the list, you have to use the get(i) methods then. It has the additional advantage that you are still type safe with generics

Siedlerchr
Siedlerchr reviewed Oct 23, 2018
View reviewed changes
src/main/java/org/jabref/gui/bibtexkeypattern/BibtexKeyPatternPanel.java Outdated
@@ -81,11 +78,11 @@ private void buildGUI() {
gridPane.add(defaultPat, 2, rowIndex);
gridPane.add(button, 3, rowIndex);

Object[] entryTypes = EntryTypes.getAllValues(mode).toArray();
List<EntryType> entryTypes = new ArrayList<>(EntryTypes.getAllValues(mode));
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 already a collection, you can just loop through it, for (EntryType type : EntryTypes.getAllValues(mode)) {
Collection is the Interface for all lists/arrays etc . The rule of thumb is to (always) code against interfaces
https://docs.oracle.com/javase/8/docs/api/java/util/Collection.html

Siedlerchr
Siedlerchr approved these changes Oct 23, 2018
View reviewed changes
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.

Thanks, looks good to me now!

@pavlapp
Copy link
Contributor Author

pavlapp commented Oct 23, 2018

Could you please explain me, why is code quality and continuous-integration always failed? In quality review shows, that there is unused import of array list, but it is not true, because I removed it.

@Siedlerchr
Copy link
Member

If you use Eclipse and have imported the project via gradlew eclipse you can do ctrl + o to automatically fix the imports.
And seems like there was an empty line between the packages which was not correct. @tobiasdiez just fixed it, so it maybe that you two got in the way

tobiasdiez
tobiasdiez requested changes Oct 23, 2018
View reviewed changes
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. Good idea to split the layout into multiple columns. The fix for the invisible scrollbar also looks good to me, but please try to generalize it so that it also applies to other dialogs. Thanks!

src/main/java/org/jabref/gui/preferences/PreferencesDialog.css Outdated
@@ -11,6 +11,15 @@
-fx-padding: 0em 1em 0em 3em;
}

.preferencePaneContainer .scroll-bar {
Copy link
Member

Choose a reason for hiding this comment

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

This fixes the problem for the preferences dialog, but the problem also appears in some other dialogs. Can you please generalize your solution (does it work without the .preferencePaneContainer selector?) and move it to the base.css. Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can you look at my solution? Is it enough generalized?

tobiasdiez
tobiasdiez approved these changes Oct 24, 2018
View reviewed changes
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 the quick follow-up!

@tobiasdiez tobiasdiez merged commit 62fb971 into JabRef:master Oct 24, 2018
Siedlerchr added a commit that referenced this pull request Oct 27, 2018
@Siedlerchr
Merge remote-tracking branch 'upstream/master' into ooPanel
3a7ef14 
* upstream/master: (30 commits)
  Update README.md (#4419)
  Fix generate bibtex key overwrite warning dialog (#4418)
  Fix group hit counter when adding entries (#4413)
  Update README.md
  fix java.nio.file.FileSystemNotFoundException and reorganize code (#4416)
  fixed an issue where corresponding groups are sometimes not highlighted when clicking on entries (#4404)
  Add a few journal abbreviations (#4412)
  Fix display of radio buttons (#4411)
  Update issue templates
  Delete ISSUE_TEMPLATE.md
  Update issue templates
  Integrate mrdlib redesign (#4361)
  fix "save" button in preference dialog not response (#4406)
  show dialog before creating entry (#4405)
  Scrollbar invisible in Preferences -> BibTex Key Pattern (#4287) (#4398)
  Updates (#4402)
  Fixes for all mods exporter tests (#4396)
  Fix EntryType dialog not closed after generate button (#4390)
  Make rank image narrower (#4395)
  Fix showing multiple icons for one menu entry - fixes #4384 (#4392)
  ...
Siedlerchr added a commit that referenced this pull request Oct 29, 2018
@Siedlerchr
Merge remote-tracking branch 'upstream/master' into docbookfive
4f21871 
* upstream/master: (54 commits)
  Update README.md (#4419)
  Fix generate bibtex key overwrite warning dialog (#4418)
  Fix group hit counter when adding entries (#4413)
  Update README.md
  fix java.nio.file.FileSystemNotFoundException and reorganize code (#4416)
  fixed an issue where corresponding groups are sometimes not highlighted when clicking on entries (#4404)
  Add a few journal abbreviations (#4412)
  Fix display of radio buttons (#4411)
  Update issue templates
  Delete ISSUE_TEMPLATE.md
  Update issue templates
  Integrate mrdlib redesign (#4361)
  fix "save" button in preference dialog not response (#4406)
  show dialog before creating entry (#4405)
  Scrollbar invisible in Preferences -> BibTex Key Pattern (#4287) (#4398)
  Updates (#4402)
  Fixes for all mods exporter tests (#4396)
  Fix EntryType dialog not closed after generate button (#4390)
  Make rank image narrower (#4395)
  Fix showing multiple icons for one menu entry - fixes #4384 (#4392)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tobiasdiez tobiasdiez tobiasdiez approved these changes

@Siedlerchr Siedlerchr Siedlerchr approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@pavlapp @Siedlerchr @tobiasdiez