Skip to content

UI: Fix for group icon#7552

Merged
tobiasdiez merged 3 commits into
JabRef:masterfrom
mohit038-zz:fix-issue-4129
Mar 20, 2021
Merged

UI: Fix for group icon#7552
tobiasdiez merged 3 commits into
JabRef:masterfrom
mohit038-zz:fix-issue-4129

Conversation

@mohit038-zz

@mohit038-zz mohit038-zz commented Mar 20, 2021 •
edited
Loading

Copy link
Copy Markdown
Contributor

Fixes #4129

Replaced the default group play icon by small circle.

Screenshot 2021-03-20 at 4 13 02 PM

I think this also fixes #7550.

Screenshot 2021-03-20 at 4 59 33 PM

Screenshot 2021-03-20 at 5 07 50 PM

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

@tobiasdiez

tobiasdiez commented Mar 20, 2021

Copy link
Copy Markdown
Member

Thanks for your PR. The problem with the default circle is that it is rather big and thus dominates the interface too much (it's pretty hard to read the group label because your eyes are constantly focusing the big colorfull blob in front of it). It would be thus nice if this circle could be scaled down (without scaling other icons, because they get too small otherwise).

@Siedlerchr

Siedlerchr commented Mar 20, 2021

Copy link
Copy Markdown
Member

Can you check if the color thing fixes #7550 as well?

@mohit038-zz

mohit038-zz commented Mar 20, 2021

Copy link
Copy Markdown
Contributor Author

@tobiasdiez
Thanks for the suggestion, I think the icon library has been updated and different sizes of circles are already present in the library.
Let me know which one do you prefer.

  1. CIRCLE_MEDIUM

Screenshot 2021-03-20 at 6 37 23 PM

  1. RECORD

Screenshot 2021-03-20 at 6 56 09 PM

@mohit038-zz

mohit038-zz commented Mar 20, 2021 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Can you check if the color thing fixes #7550 as well?

I checked it and also updated the screenshots in #7552 (comment).
It fixes all issues mentioned in #7550.

@Siedlerchr

Siedlerchr commented Mar 20, 2021

Copy link
Copy Markdown
Member

Cool! For me RECORD looks appropriate

@tobiasdiez

tobiasdiez commented Mar 20, 2021

Copy link
Copy Markdown
Member

That's awesome. I agree, record looks the best.

tobiasdiez
tobiasdiez reviewed Mar 20, 2021
View reviewed changes
Comment thread src/main/java/org/jabref/gui/icon/InternalMaterialDesignIcon.java Outdated
FontIcon fontIcon = FontIcon.of(icon);
fontIcon.getStyleClass().add("glyph-icon");
color.ifPresent(fontIcon::setIconColor);
color.ifPresent(color -> fontIcon.setStyle(fontIcon.getStyle() + String.format("-fx-fill: %s;", ColorUtil.toRGBCode(color))));

@tobiasdiez tobiasdiez Mar 20, 2021

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Do you know why setIconColor doesn't work? Maybe add a short comment so that we will not revert it in the future by accident.

@mohit038-zz mohit038-zz Mar 20, 2021

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Sorry, I have no idea.

@Siedlerchr Siedlerchr Mar 20, 2021

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It has something to do with the style class. If you uncomment the line "getStyleClass.add("glyph-icon")" than it works properly. However, then the rest of the icons are black because that property is set in the css (and is obviously different for dark and base.css) and therefore overrides the color of the 'FontIcon".

So add a comment like:
Override the default color from the css files

@Siedlerchr Siedlerchr added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Mar 20, 2021
@tp-1000 tp-1000 mentioned this pull request Mar 20, 2021
Closed
1 task
@tobiasdiez

tobiasdiez commented Mar 20, 2021

Copy link
Copy Markdown
Member

Thanks again!

@tobiasdiez tobiasdiez merged commit d31ef4d into JabRef:master Mar 20, 2021
@mohit038-zz mohit038-zz deleted the fix-issue-4129 branch March 20, 2021 23:47
Siedlerchr added a commit that referenced this pull request Mar 28, 2021
@Siedlerchr
Merge remote-tracking branch 'upstream/master' into writebackupToTemp
f99ba6f 
* upstream/master: (191 commits)
  Fix for issue 7416: font size of the preferences dialog does not update with the rest of the GUI. (#7509)
  Fix school/instituation is printed twice (#7574)
  Dsiable notarisation until we hae an account for JabRef e.V. (#7572)
  Fix citation keys unintentionally being overwritten on import (#7443)
  Fix AuthentificationPlugin not declared in mergedModule (#7570)
  Suggestions for changes in caching latex free authors (#7301)
  Add simple Unit Tests (#7542)
  Fix drag and drop into empty library (#7555)
  Bump richtextfx from 0.10.4 to 0.10.6 (#7563)
  Bump pdfbox from 2.0.22 to 2.0.23 (#7561)
  Bump org.eclipse.jgit (#7560)
  Bump fontbox from 2.0.22 to 2.0.23 (#7562)
  Bump guava from 30.1-jre to 30.1.1-jre (#7564)
  Bump xmpbox from 2.0.22 to 2.0.23 (#7565)
  Bump hmarr/auto-approve-action from v2.0.0 to v2.1.0 (#7566)
  Add gource (#7193)
  UI: Fix for group icon (#7552)
  Fix for issue 6487: Opening BibTex file (doubleclick) from Folder with spaces not working (#7551)
  add ability to insert arxivId (#7549)
  Fixed missing trigger for linked file operations (#7548)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tobiasdiez tobiasdiez tobiasdiez left review comments

@Siedlerchr Siedlerchr Siedlerchr approved these changes

Assignees

No one assigned

Labels

status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Monocolor for icons Replace default group icon by small circle

4 participants

@mohit038-zz @tobiasdiez @Siedlerchr @mohit038