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

Update main table colors #11579

Merged
merged 13 commits into from
Aug 6, 2024
Merged

Update main table colors #11579

merged 13 commits into from
Aug 6, 2024

Conversation

LoayGhreeb
Copy link
Collaborator

Follow-up to: #11510. Closes LoayGhreeb#8.
Before:
image

After:
image

Mandatory checks

  • Change in CHANGELOG.md described in a way that is understandable for the average user (if applicable)
  • Tests created for changes (if applicable)
  • Manually tested changed features in running JabRef (always required)
  • Screenshots added in PR description (for UI changes)
  • Checked developer's documentation: Is the information available and up to date? If not, I outlined it in this pull request.
  • Checked documentation: Is the information available and up to date? If not, I created an issue at https://github.com/JabRef/user-documentation/issues or, even better, I submitted a pull request to the documentation repository.

@@ -649,15 +649,27 @@ TextFlow > .tooltip-text-monospaced {
}

.table-row-cell:matching-search-and-groups {
-fx-background-color: white;
-fx-background-color: #FFFFFFFF;
Copy link
Member

Choose a reason for hiding this comment

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

Please either make them derived or add some new constants, as this makes it easier for dark theme to change the colors

@subhramit subhramit added the ui label Aug 5, 2024
@koppor
Copy link
Member

koppor commented Aug 5, 2024

Carl and I are on it.

@koppor
Copy link
Member

koppor commented Aug 5, 2024

Using this gray-blue colors looks odd:

image

@koppor
Copy link
Member

koppor commented Aug 5, 2024

Blue berry:

image

@koppor
Copy link
Member

koppor commented Aug 5, 2024

With -jr-theme:

image

@koppor
Copy link
Member

koppor commented Aug 5, 2024

With -jr-theme, memories of the C64 come back. I "strictly" used -jr-theme color 😇

@koppor
Copy link
Member

koppor commented Aug 5, 2024

I added src/test/resources/testbib/simple-search-library.bib to be able to display all 4 cases on a single page.

  • Search for S
  • Selectd group g

Then, Sg matches both, S only the search string, g only group, and n` none of them.

calixtus
calixtus previously approved these changes Aug 5, 2024
@calixtus calixtus enabled auto-merge August 5, 2024 22:43
@LoayGhreeb
Copy link
Collaborator Author

LoayGhreeb commented Aug 5, 2024

Double-clicking on an even row removes the selection color.


Edit: If there is no active search query and the 'All Entries' group is selected, the odd rows keeps the selection color. However, if there is an active search or a selected group, all rows will lose the selection color when double-clicked.

@LoayGhreeb
Copy link
Collaborator Author

The text is not shown on the selected non-matched entry.
image

@LoayGhreeb LoayGhreeb disabled auto-merge August 5, 2024 22:50
@koppor
Copy link
Member

koppor commented Aug 5, 2024

However, if there is an active search or a selected group, all rows will lose the selection color when double-clicked.

Single click works. What magic does JabRef do on a double click?

@LoayGhreeb
Copy link
Collaborator Author

The only action on double-click is to open the entry editor:

.withOnMouseClickedEvent((entry, event) -> {
if (event.getClickCount() == 2) {
libraryTab.showAndEdit(entry.getEntry());
}
})

@koppor
Copy link
Member

koppor commented Aug 5, 2024

If we cannot fix the next 20 hours, we merge and create two follow-up issues. - Without stripes, it looks really bad IMHO.

@LoayGhreeb
Copy link
Collaborator Author

Sorry for the confusion in #11579 (comment), but the issue only affects the even rows. Odd rows keep the selection color in all cases.

@koppor
Copy link
Member

koppor commented Aug 6, 2024

I cannot manage to "double filter":

.table-row-cell:not(.focused):matching-groups-not-search > .table-cell {
    -fx-text-fill: -jr-match-3-text-color;
}

That would be requried to have the text color only applied when not focused - but does not trigger at all.

Seems to be known issue - https://stackoverflow.com/q/39355081/873282

Thus, added a work around at e7c0460 (#11579).

@koppor
Copy link
Member

koppor commented Aug 6, 2024

The mentioned issues should be fixed now.

@LoayGhreeb LoayGhreeb enabled auto-merge August 6, 2024 00:29
Copy link
Contributor

github-actions bot commented Aug 6, 2024

The build for this PR is no longer available. Please visit https://builds.jabref.org/main/ for the latest build.

@LoayGhreeb LoayGhreeb added this pull request to the merge queue Aug 6, 2024
Merged via the queue into main with commit 21f0f22 Aug 6, 2024
22 checks passed
@LoayGhreeb LoayGhreeb deleted the updateTableColors branch August 6, 2024 07:58
@koppor koppor mentioned this pull request Aug 6, 2024
6 tasks
@koppor
Copy link
Member

koppor commented Sep 30, 2024

Other color scheme PR: #3839

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bring back stripes
5 participants