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

Change blue and red colors in Merge entries dialog in Dark theme #6356

Merged
merged 2 commits into from
Apr 26, 2020

Conversation

MootezSaaD
Copy link
Contributor

This PR is related to #6334 (blue text color in the Merge entries dialog in dark theme).
After modifications, the dialog looks like this
image

I have also tried to replace the red color upon @koppor 's comment (located in the aforementioned issue)

  • Change in CHANGELOG.md described (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 documentation: Is the information available and up to date? If not created an issue at https://github.com/JabRef/user-documentation/issues or, even better, submitted a pull request to the documentation repository.

@Siedlerchr
Copy link
Member

Thanks for tackling this issue. I am not happy about that blue in general. Maybe a better contrast color e.g. a blue with #54A3F2

See for example here for a combination of possible colors:
https://app.contrast-finder.org/result.html?foreground=%234281BE&background=%23272B38&ratio=4.5&isBackgroundTested=false&algo=HSV

@MootezSaaD
Copy link
Contributor Author

I think #54A3F2 seems to be more suitable
image

@Siedlerchr
Copy link
Member

Yep, that blue looks good! I also checked the red color, this one should have better contrast as well:
#FA5B68 Could you also show a screenshot of both new colors?

@MootezSaaD
Copy link
Contributor Author

MootezSaaD commented Apr 26, 2020

Both colors ( #54A3F2 and #FA5B68 ) seem more visible to me

image

@Siedlerchr Siedlerchr requested a review from koppor April 26, 2020 17:48
@Siedlerchr
Copy link
Member

Yes, that looks good to mew now! Go ahead!

@koppor
Copy link
Member

koppor commented Apr 26, 2020

Awesome. Thank you for the quick action taken and the fast replies to the reviews @MootezSaaD. @Siedlerchr Thank you for the quick and constructive review, too!

@koppor koppor merged commit ce05e1e into JabRef:master Apr 26, 2020
@Siedlerchr
Copy link
Member

@MootezSaaD Maybe you can fix some other issues with the dark theme as well?
This would be really cool! See #5522 for a list of issues

@MootezSaaD
Copy link
Contributor Author

Absolutely, I'll give it a look

Siedlerchr added a commit that referenced this pull request Apr 30, 2020
…ionCaseInsensitive

* upstream/master:
  New Crowdin translations (#6375)
  Squashed 'src/main/resources/csl-styles/' changes from 143464e..906cd6d
  Fixes #6357: File directory (#6377)
  Disable the generate button if the ID field is empty (#6371)
  Fix Preferences style value too long (#6372)
  Fix various Dark theme issues (#6368)
  Correct label name in dependabot
  Bump java-diff-utils from 4.5 to 4.7 (#6365)
  Try with info.plist.template also (#6366)
  Fix wrong button order (Apply and Cancel) in ManageProtectedTermsDialog. (#6358)
  Bump flexmark-ext-gfm-strikethrough from 0.61.6 to 0.61.20 (#6361)
  Bump checkstyle from 8.31 to 8.32 (#6360)
  Bump flexmark-ext-gfm-tasklist from 0.61.16 to 0.61.20 (#6364)
  Bump flexmark from 0.61.16 to 0.61.20 (#6359)
  Bump org.beryx.jlink from 2.17.7 to 2.17.8 (#6362)
  Bump classgraph from 4.8.71 to 4.8.77 (#6363)
  Change blue and red colors in Merge entries dialog in Dark theme (#6356)
  Add Info plist to mac resources (#5986)
  Backward compatibility for 4.3.1 (#6296)
koppor pushed a commit that referenced this pull request Mar 1, 2023
e9fd2027de Add Medicine Publishing Styles (#6434)
cae128f35f Create Bristol University Press (#6356)
74b4af3b82 Create internet-archaeology.csl (#6357)
ee7ece480b Add Bio-Protocol style (#6429)
9a455efcee Create archives-of-medical-research.csl (#6415)
e91aba46fc Remove some bursa-uludag styles (#6423)
03f3962657 Update offa.csl (#6428)
95dc9b9f5a Update journal-of-neolithic-archaeology.csl (#6427)
a4e6c7f477 Update the-university-of-winchester-harvard.csl (#6374)
c0bf10647a add manuscript formatting to ASA (#6387)
3a673a564a Update universite-de-sherbrooke-histoire.csl (#6392)
0c48c7289e Update chemistry-education-research-and-practice.csl (#6397)
51f718a7b9 Update journal-of-endodontics.csl (#6409)
51e419051f Update presses-universitaires-de-rennes.csl (#6413)

git-subtree-dir: buildres/csl/csl-styles
git-subtree-split: e9fd2027de4e2355f3244ac662960467e225774d
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants