Dark theme: Entry table: Header of index without the proper color

[mlep]

JabRef 5.1--2020-08-26--fd5bc52
Linux 4.9.0-13-amd64 amd64
Java 15-ea

• Mandatory: I have tested the latest development version from http://builds.jabref.org/master/ and the problem persists

A tiny issue: the header of the index column (with the symbol #) does not follow the color scheme of the dark theme. The symbol is black while it should be gray.
See top left-hand corner:

[vatsal-M]

Our team @ISD14-LNMIIT wants to work on this issue. Please assign this issue to us!
@Siedlerchr

[Siedlerchr]

Thanks for taking this issue! At the same time you could also take a look at #6791 also an issue with the dark theme. Probably some color definitions confused.

For getting started please read through our Contribution guide
Codewise the DarkTheme is located here:
https://github.com/JabRef/jabref/blob/master/src/main/java/org/jabref/gui/Dark.css

[vatsal-M]

Surely, our team will loook into this issue too

[Gena928]

@Siedlerchr,
if possible, I would like to offer 2 solutions for this issue:

1. Convert column into "field column".
   Column header will change its color as required. But we are loosing a tooltip - "Index". I noticed, that all other columns with text in header do not contain tooltips. So, maybe it's OK.

2. Change column header from "#" into an icon.
   In this case we are not loosing tooltip. Example:

Unfortunately I can't keep the header text "#" AND tooltip at the same time. )))

[Siedlerchr]

@Gena928 Thanks for taking this.
I am actually not that deep into the main table column model. Maybe @calixtus or @tobiasdiez can say more

[tobiasdiez]

Another idea would be to add a style class to the node in https://github.com/JabRef/jabref/blob/master/src/main/java/org/jabref/gui/maintable/MainTableColumnFactory.java#L132 which sets the right font color.

[Gena928]

@tobiasdiez
thanks for Idea. I found a stylesheet Dark.css and feel like .text-unchanged is what we need.
header.getStyleClass().add("text-unchanged");

Can I create a pull request?

[Siedlerchr]

@Gena928 Sure, go ahead! Another issue with the Dark Theme was reported here, maybe you can also take a look at that #6791

[calixtus]

Are there any examples where "text-unchanged" is also used?

[Gena928]

I found only one more entry in package org.jabref.gui.mergeentries.
Class DiffHighlighting.
Method public static Text forUnchanged(String text)

[IsaacRoles]

I was able to solve the same issue with these small changes to:
https://github.com/JabRef/jabref/blob/master/src/main/java/org/jabref/gui/maintable/MainTableColumnFactory.java#L132
and
https://github.com/JabRef/jabref/blob/master/src/main/java/org/jabref/gui/icon/IconTheme.java

I'm not sure which is the better solution.

The methods createIndexColumn and createGroupColumn work very differently from the other column creation methods (for example createFilesColumn). They should possibly be reworked completely to be more consistent with the rest of the design.

[calixtus]

I found only one more entry in package org.jabref.gui.mergeentries.
Class DiffHighlighting.
Method public static Text forUnchanged(String text)

Thank you all for your proposals, but I don't think that either one is a real solution for this issue.

As the css-class text-unchanged for the header is used for something completly unrelated to the header text color, following this proposal would result in strange behaviour, if someone later decides to change the color of text-unchanged to something else, if he want's to change the color of unchanged texts in the diff/merge dialog. I think a better solution would then either be to introduce another css-class for main table headings or to debug, why the standard color is not applied in the main table headers.

The other solution to use an icon would work in this case, but I think it's more a workaround but a solution. We would end up again in this issue as soon as someone tries to add another column with a text header.

The methods createIndexColumn and createGroupColumn work very differently from the other column creation methods (for example createFilesColumn). They should possibly be reworked completely to be more consistent with the rest of the design.

In fact they work pretty much the same, but a few columns were outsourced (in the literal meaning) to new packages, to make the factory class a bit more easy to read. But you can move the the logic of createIndexColumn and the createGroupColumn to their own packages / classes, if you think it's worth it. Should be a nice little programming exercise.

[tobiasdiez]

I agree completely with @calixtus. I think, a new css class for the header would be the best solution.

[Gena928]

Good day everyone.
If possible, I would like to get your advice, before creating a pull request.
I've added CSS class to Dark.css & Base.css:

.mainTable-header{
    -fx-fill: -fx-mid-text-color;
}

Then I added this css class to MainTableColumnFactory.java After line 132

header.getStyleClass().add("mainTable-header");

So does it looks like correct solution?

[calixtus]

Sounds reasonable. Does it work?
If it's the same for dark.css and for base.css it should be sufficient to add it to base.css, as dark.css only makes some modification to the base.css, which is always loaded.

[Gena928]

@calixtus
thanks for comment. I removed CSS class from Dark.css and the picture is still OK.

So now I have only this in Base.css

.mainTable-header{
    -fx-fill: -fx-mid-text-color;
}

and this in MainTableColumnFactory.java
header.getStyleClass().add("mainTable-header");

[Siedlerchr]

@Gena928 Yes, that's good, so go ahead and crate your PR!

[calixtus]

Looks good. Can you please also check some of the dialogs that use a table that they are fine too?

[Gena928]

Sorry, can't find them. )) I see that MainTable can be created only via BasePanel.getMainTable.
When I trace usage of BasePanel.getMainTable I see that only 2 methods are using it:

• PreferencesDialogViewModel (refresh method of MainTable)
• GlobalSearchBar (selectFirst method of MainTable);

Sorry, are there any dialogs, that use MainTable?

[calixtus]

Sorry, can't find them. )) I see that MainTable can be created only via BasePanel.getMainTable.
When I trace usage of BasePanel.getMainTable I see that only 2 methods are using it:

* PreferencesDialogViewModel (refresh method of `MainTable`)

* GlobalSearchBar (selectFirst method of `MainTable`);

Sorry, are there any dialogs, that use MainTable?

Ooops, did not think before writing. Sorry. :-D

[mlep]

@Gena928 Thank you for fixing this!