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

Make global font size customizable #4178

Merged
merged 5 commits into from
Jul 12, 2018
Merged

Make global font size customizable #4178

merged 5 commits into from
Jul 12, 2018

Conversation

tobiasdiez
Copy link
Member

Font size customization was removed in #4109 since it didn't work properly (i.e. only some elements of the ui were scaled). With this PR the global font size can be controlled by the user (in the Appearance tab of the preference dialog) and changes there affect every JavaFX control.


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

@tobiasdiez tobiasdiez added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Jul 7, 2018
int newFontSize = Integer.parseInt(fontSize.getText());
prefs.putInt(JabRefPreferences.MAIN_FONT_SIZE, newFontSize);
isRestartRequired |= oldOverrideDefaultFontSize != overrideFonts.isSelected();
isRestartRequired |= oldFontSize != newFontSize;

Copy link
Member

Choose a reason for hiding this comment

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

Is this a binary or? This one looks confusing

Copy link
Member Author

Choose a reason for hiding this comment

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

yes, it is (bit-wise) or paired with assignment (similar to +=). See https://stackoverflow.com/questions/14295469/what-does-mean-pipe-equal-operator

Copy link
Member

Choose a reason for hiding this comment

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

Can you please find another abstraction for solving this functionality? I know it's elegant, but I would not consider this as appropriate Java code within JabRef.

Copy link
Member

Choose a reason for hiding this comment

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

Agree with Linus here, it's not intuitive and easy to read.

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 also agree ;-)

Copy link
Member

@LinusDietz LinusDietz left a comment

Choose a reason for hiding this comment

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

I'm okay if I'm outvoted on this issue by the @JabRef/developers, but I think that binary fiddling is not okay.

int newFontSize = Integer.parseInt(fontSize.getText());
prefs.putInt(JabRefPreferences.MAIN_FONT_SIZE, newFontSize);
isRestartRequired |= oldOverrideDefaultFontSize != overrideFonts.isSelected();
isRestartRequired |= oldFontSize != newFontSize;

Copy link
Member

Choose a reason for hiding this comment

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

Can you please find another abstraction for solving this functionality? I know it's elegant, but I would not consider this as appropriate Java code within JabRef.

@tobiasdiez tobiasdiez merged commit dd9b229 into master Jul 12, 2018
@tobiasdiez tobiasdiez deleted the changeFontSize branch July 12, 2018 20:26
@stefan-kolb stefan-kolb mentioned this pull request Jul 13, 2018
8 tasks
Siedlerchr added a commit that referenced this pull request Jul 26, 2018
* upstream/master: (47 commits)
  Make attached files relative to the file directory (#4212)
  execute set visible in swing thread to avoid blocking
  Fix isbn chimbori test (#4234)
  Rewrite web search pane in JavaFX (#4203)
  Update dependencies (#4231)
  Update to latest release of richtextfx (#4213)
  Fix fetcher tests (#4216)
  single line text fields (#4138)
  Make it easier to rename and move files (#4200)
  Fix that swing dialogs are hidden behind main window (#4205)
  Fixed: #4166 add move & rename file (#4217)
  update gradle plugins and gradlen to 4.9
  Update dependencies
  Remove unnecessary look and feel migration (#4204)
  Revert threading fix since this sometimes lead to freezes...
  Update year
  Fix a few more threading exceptions
  Refactor
  Convert CiteSeerX fetcher to new infrastructure (#4185)
  Make global font size customizable (#4178)
  ...

# Conflicts:
#	src/main/java/org/jabref/gui/BasePanel.java
#	src/main/java/org/jabref/gui/JabRefFrame.java
#	src/main/java/org/jabref/gui/mergeentries/MergeEntries.java
#	src/main/resources/l10n/JabRef_en.properties
@Siedlerchr Siedlerchr mentioned this pull request May 18, 2019
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.

4 participants