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

Fix for Dark theme not readable #8929

Merged
merged 5 commits into from
Jun 27, 2022
Merged

Fix for Dark theme not readable #8929

merged 5 commits into from
Jun 27, 2022

Conversation

victordmp
Copy link
Contributor

@victordmp victordmp commented Jun 26, 2022 •
edited
Loading

Initial Proposed

  • Change the text color of the summary that was previously black. In order to improve the contrast in dark mode theme.

My Proposed

  • Change the color of the abstract text from black to a lighter color. In order to improve the contrast in the dark mode theme.
  • Change the background color of .list-cell:entry-selected to a darker color. In order to improve the contrast with the color of the title and abstract.

Fixes #7927

In the dialog for importing entries, both the title and the summary value were difficult to read in Dark Theme mode. To solve this problem I did the following

  • Fixed the background of the text selected in the ImportEntriesDialog.

    • Before:
      Screen Shot 2022-06-26 at 13 40 29
    • After:
      Screen Shot 2022-06-26 at 13 33 57

  • Corrected the unselected text color of the summary in the Import Entries Dialog.

    • Before:
      Screen Shot 2022-06-26 at 13 33 57
    • After:
      Screen Shot 2022-06-26 at 13 37 53

Checking the changes

  1. Have Dark Theme enabled
  2. Search for an entry in the web search
  3. Import entries window opens

PR requirements

  • 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.

@Siedlerchr Siedlerchr changed the title Fix for issue 7927 Fix for Dark theme not readable Jun 26, 2022
@Siedlerchr Siedlerchr added status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers ui labels Jun 26, 2022
koppor
koppor requested changes Jun 26, 2022
View reviewed changes
Copy link
Member

@koppor koppor left a comment

Choose a reason for hiding this comment

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

Thank you for the PR! I am always using the light theme. I switched to the dark theme - and it looks OK enough.

For future work, we should try to look at the color theme of Visual Studio Code. It has a very decent dark mode. (This also refs the discussion at #7825 (comment))

src/main/java/org/jabref/gui/Base.css Outdated Show resolved Hide resolved
src/main/java/org/jabref/gui/importer/ImportEntriesDialog.css Outdated Show resolved Hide resolved
src/main/java/org/jabref/gui/importer/ImportEntriesDialog.css Outdated Show resolved Hide resolved
src/main/java/org/jabref/gui/importer/ImportEntriesDialog.css Outdated Show resolved Hide resolved
@victordmp
Copy link
Contributor Author

Thank you for the PR! I am always using the light theme. I switched to the dark theme - and it looks OK enough.

For future work, we should try to look at the color theme of Visual Studio Code. It has a very decent dark mode. (This also refs the discussion at #7825 (comment))

Thanks for the feedback. Threads solved.

@Siedlerchr
Copy link
Member

Thanks, lgtm! Can you please add changelog entry as well?

@calixtus
Copy link
Member

Changes look good so far. Could you please double check if there isn't any compareable ui situation in JabRef where text is displayed on highlighted background?
I wonder if we should not choose a name a bit more generic, like 'highlighted-text-fill' or something... Opinions?

@victordmp
Copy link
Contributor Author

victordmp commented Jun 27, 2022 •
edited
Loading

Thanks, lgtm! Can you please add changelog entry as well?

Thanks, I will update the changelog.

@victordmp
Copy link
Contributor Author

victordmp commented Jun 27, 2022 •
edited
Loading

Changes look good so far. Could you please double check if there isn't any compareable ui situation in JabRef where text is displayed on highlighted background? I wonder if we should not choose a name a bit more generic, like 'highlighted-text-fill' or something... Opinions?

Thanks for the feedback.

About the first question, I did a search for the word summary in the repository. This word represents the class that has been changed. Only one place uses this style class, located in the file . The file ImportEntriesDialog.java imports the style class summary as follows .getStyleClass().add("summary") (line 172).

This Dialog is exactly what we wanted to make the change to. In answer to the second question, I think the name you propose (highlighted-text-fill) is really interesting.

@victordmp
Copy link
Contributor Author

Thanks, lgtm! Can you please add changelog entry as well?

Changelog updated.

@Siedlerchr
Copy link
Member

Thanks again for the quick follow up!

@Siedlerchr Siedlerchr merged commit 586d3e4 into JabRef:main Jun 27, 2022
@victordmp
Copy link
Contributor Author

Thanks again for the quick follow up!

You are welcome :)

Siedlerchr added a commit to LIM0000/jabref that referenced this pull request Jun 28, 2022
@Siedlerchr
Merge remote-tracking branch 'upstream/main' into fix-for-issue-8911
742d320 
* upstream/main:
  Fix for Dark theme not readable (JabRef#8929)
  Find Unlinked files should ignore Thumbs.db, etc (JabRef#8800)
  Try to  make reproducible builds (JabRef#8925)
  Link to new board (and refine text)
  Pass the data as a string (JabRef#8923)
  Add IntelliJ plugins to Gitpod configuration
  Add gradle support
  Fix extension name
  Remove obsolete closing bracket
  Add gitpod config (JabRef#8921)
  Fix javafx version not displayed in About Dialog (JabRef#8918)
  Redesign "Manage field names & content" dialog (JabRef#8892)
Siedlerchr added a commit that referenced this pull request Jul 1, 2022
@Siedlerchr
Merge remote-tracking branch 'upstream/main' into controlsfx-tags
ba3ffce 
* upstream/main:
  fix merge conflict
  Squashed 'buildres/csl/csl-locales/' changes from 969d9567ac..55459cd79f
  Squashed 'buildres/csl/csl-styles/' changes from e740261..3d3573c
  Show pdf icon for mime type in maintable (#8935)
  Fix for Dark theme not readable (#8929)
  Find Unlinked files should ignore Thumbs.db, etc (#8800)
  Try to  make reproducible builds (#8925)
  Link to new board (and refine text)
  Pass the data as a string (#8923)
  Add IntelliJ plugins to Gitpod configuration
  Add gradle support
  Fix extension name
  Remove obsolete closing bracket
Siedlerchr added a commit to LIM0000/jabref that referenced this pull request Jul 9, 2022
@Siedlerchr
Merge remote-tracking branch 'upstream/main' into fix-for-issue-8011
88f8983 
* upstream/main:
  Disable ResearchGateTest on CI (JabRef#8955)
  Bump checkstyle from 10.1 to 10.3.1 (JabRef#8950)
  Bump checkstyle from 10.1 to 10.3.1 (JabRef#8950)
  Bump mariadb-java-client from 2.7.5 to 2.7.6 (JabRef#8953)
  Add notification when adding previous entries to new group configuration (JabRef#8919)
  Remember Sidepane width after restart (JabRef#8936)
  move "Warn about duplicates on import" preferences option (JabRef#8937)
  Fix theme switching exception (JabRef#8939)
  fix merge conflict
  Squashed 'buildres/csl/csl-locales/' changes from 969d9567ac..55459cd79f
  Squashed 'buildres/csl/csl-styles/' changes from e740261..3d3573c
  Show pdf icon for mime type in maintable (JabRef#8935)
  Fix for Dark theme not readable (JabRef#8929)
  Find Unlinked files should ignore Thumbs.db, etc (JabRef#8800)
  Try to  make reproducible builds (JabRef#8925)
  Link to new board (and refine text)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@koppor koppor koppor requested changes

@Siedlerchr Siedlerchr Siedlerchr approved these changes

Assignees
No one assigned
Labels
import status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers ui
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Dark Theme : Search -> Import entries dialog text not readable
6 participants
@victordmp @Siedlerchr @calixtus @koppor @ThiloteE @euEmica