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

Table row height adjusts on Windows as the font scales with the menu #1623

Merged
merged 4 commits into from
Aug 17, 2016

Conversation

oscargus
Copy link
Contributor

When increasing the menu font size on Windows (at least WIndows 10), the tables also scale but not the row width. This has been fixed in some dialogs earlier, but this is a general fix.

I have no idea how it works on OSX, but on Linux (my CentOS office machine) it seems like the table font is not the same as the menu font.

This is relevant for high resolution displays (although I cannot find the issue that discussed it...)

Before:
capture8

After:
capture9

  • Change in CHANGELOG.md described
  • Screenshots added (for bigger UI changes)

@oscargus oscargus added type: enhancement ui status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers labels Jul 24, 2016
@simonharrer
Copy link
Contributor

👍 Really nice. What I am concerned about is the number of places we have to put this check. We should really think of a way where configuration changes can flow from the preferences to the GUI elements. Maybe use a event bus for the preferences as well, especially regarding the classes in the GUI package? This could maybe reduce the number of restarts that are required upon changing preferences. What do the other developers think? @JabRef/developers

@oscargus
Copy link
Contributor Author

It is a bit weird that the rows are not automatically resized on Windows. It seems like this happens for the heading but not the content rows. Maybe easier to send in a bug report to Oracle/Microsoft?

@Siedlerchr
Copy link
Member

We are not the first who stumble across this issue. And I doubt that it will be fixed for Swing.
Fpr high dpi things, a bit searching on SO found that source, which compares different sizes and dpi settings, including JavaFX:
http://kynosarges.org/GuiDpiScaling.html

@oscargus
Copy link
Contributor Author

oscargus commented Jul 25, 2016 via email

@oscargus
Copy link
Contributor Author

oscargus commented Jul 25, 2016 via email

@matthiasgeiger
Copy link
Member

As there was recently a issue or forum discussion regarding this topic I just want to make sure that you are aware of two different configurations: There is the option to increase/decrease the table font size using the "View" menu and there is also the "Menu and label font size" in Preferences -> Appearance. The latter setting not only affects the "Menu and label font size" but also sets the font size in the entry editor and potentially at other places...

And I just saw that there is also a "Table row height padding" setting which might be reason for the strange height behavior...?

@oscargus
Copy link
Contributor Author

The current behavior is affected by "Menu and label font size".

As far as I am concerned "Table row height padding" is only for the main entry table, as well as the explicit font setting and increase/decrease font size.

In the link there are arguments that this is a feature, so they will indeed not change it. (It appears as if the "ideal" default table row height is different on different OS to start with, so they decided to just use the same constant for all systems...)

@oscargus oscargus removed the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Aug 2, 2016
@oscargus oscargus force-pushed the tablepadding branch 2 times, most recently from a7b2659 to f660186 Compare August 15, 2016 21:37
@oscargus
Copy link
Contributor Author

Found a consistent way to do this. It also applied for JTree. I think that the "global" approach is Look-and-feel-dependent so this seems to be the best way to do it.

@oscargus oscargus added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Aug 15, 2016
@@ -64,6 +65,10 @@ public ChangeDisplayDialog(JFrame owner, final BasePanel panel,
}
tree = new JTree(root);
tree.addTreeSelectionListener(this);
// Fix tree row height
FontMetrics metrics = tree.getFontMetrics(tree.getFont());
Copy link
Member

Choose a reason for hiding this comment

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

Can these two lines be separated into a utility method, which gets tree passed? (At the other places, the appropriate other object; table, ...)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I realized that as well after changing everywhere... ;-)

@koppor
Copy link
Member

koppor commented Aug 17, 2016

LGTM. Please extract the two lines of code to a separate method and then it's good to go (from my side 😇).

*
* @param table
*/
public static void correctRowHeight(JTable table) {
Copy link
Member

Choose a reason for hiding this comment

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

Is it also possible to pass JComponent and merge this method with correctRowHeight(JTree tree)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No. JComponent doesn't have a setRowHeight method (I checked it before writing the code) and I do not think it is possible to cast from JTree to JTable or the opposite.

@koppor koppor merged commit 1af96f3 into JabRef:master Aug 17, 2016
Siedlerchr added a commit to Siedlerchr/jabref that referenced this pull request Aug 18, 2016
* master:
  Fix imports
  Rename NewFileDialog -> FileDialog
  Also cancel duplicate finder workflow on close button
  Removed/moved preferences which are constants
  implements JabRef#1767: Add Help Button to access new help page
  Fixed BibTeXMLImporter
  Set more default file filters in dialogs JabRef#1763
  Resolve crossrefs and strings in main table (JabRef#1644)
  Rewrite bibtexml importer with JAXB parser (JabRef#1666)
  Moved a few more initialization to GUIGlobals.init() (JabRef#1756)
  Added program to generate a table of all characters and fixed some characters (JabRef#1766)
  Improve focus of the maintable after a sidepane gets closed (JabRef#1741)
  Table row height adjusts on Windows as the font scales with the menu (JabRef#1623)
  Added more characters to converters (JabRef#1761)
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 type: enhancement ui
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants