Skip to content

Fix showing multiple icons for one menu entry - fixes #4384#4392

Merged
tobiasdiez merged 2 commits into
JabRef:masterfrom
topalavlad:allow_multiple_icons_for_menu_entry
Oct 20, 2018
Merged

Fix showing multiple icons for one menu entry - fixes #4384#4392
tobiasdiez merged 2 commits into
JabRef:masterfrom
topalavlad:allow_multiple_icons_for_menu_entry

Conversation

@topalavlad

@topalavlad topalavlad commented Oct 20, 2018

Copy link
Copy Markdown
Contributor

It currently only applies to the Rank menu; the sub-entries were only showing the first star instead of 5 stars

Since SpecialFieldMenuAction has been removed and in RightClickMenu the following has been removed, the icons are not set correctly anymore.

    public static void populateSpecialFieldMenu(JMenu menu, SpecialField field, JabRefFrame frame) {
        SpecialFieldViewModel viewModel = new SpecialFieldViewModel(field);
        menu.setText(viewModel.getLocalization());
        menu.setIcon(viewModel.getRepresentingIcon());
        for (SpecialFieldValue val : field.getValues()) {
            menu.add(new SpecialFieldMenuAction(new SpecialFieldValueViewModel(val), frame));
        }
    }

The new way of working is through ActionFactory which uses setGraphic() instead:

    private static void setGraphic(MenuItem node, Action action) {
        node.graphicProperty().unbind();
        action.getIcon().ifPresent(icon -> node.setGraphic(icon.getGraphicNode()));
    }

Since the only composite icons are the ones used for rank, using the full unicode field of Node instead of only the unicode of the first icon won't affect other Actions.

    @Override
    public Node getGraphicNode() {
        GlyphIcons icon = icons.get(0);

        Text text = new Text(icon.unicode()); //Replaced with unicode
        text.getStyleClass().add("glyph-icon");
        text.setStyle(String.format("-fx-font-family: %s;", icon.fontFamily()));

        color.ifPresent(color -> text.setStyle(text.getStyle() + String.format("-fx-fill: %s;", ColorUtil.toRGBCode(color))));
        return text;
    }

@topalavlad
Fix showing multiple icons for one menu entry - fixes #4384
3ab05f9 
It currently only applies to the Rank menu; the sub-entries were only showing the first star instead of 5 stars
tobiasdiez
tobiasdiez approved these changes Oct 20, 2018
View reviewed changes

@tobiasdiez tobiasdiez left a comment

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.

Thanks a lot for your contribution and the detailed description of the change/your pull request. Looking forward to see more PRs from you ;-)

Comment thread CHANGELOG.md Outdated
Siedlerchr
Siedlerchr approved these changes Oct 20, 2018
View reviewed changes

@Siedlerchr Siedlerchr left a comment

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.

Yeah! Thanks for your contribution! 👍

@tobiasdiez tobiasdiez merged commit e76a287 into JabRef:master Oct 20, 2018
@topalavlad topalavlad deleted the allow_multiple_icons_for_menu_entry branch October 21, 2018 06:26
Siedlerchr added a commit that referenced this pull request Oct 22, 2018
@Siedlerchr
Merge remote-tracking branch 'upstream/master' into fixgenerateButton
b7deca8 
* upstream/master:
  Fix showing multiple icons for one menu entry - fixes #4384 (#4392)
Siedlerchr added a commit that referenced this pull request Oct 27, 2018
@Siedlerchr
Merge remote-tracking branch 'upstream/master' into ooPanel
3a7ef14 
* upstream/master: (30 commits)
  Update README.md (#4419)
  Fix generate bibtex key overwrite warning dialog (#4418)
  Fix group hit counter when adding entries (#4413)
  Update README.md
  fix java.nio.file.FileSystemNotFoundException and reorganize code (#4416)
  fixed an issue where corresponding groups are sometimes not highlighted when clicking on entries (#4404)
  Add a few journal abbreviations (#4412)
  Fix display of radio buttons (#4411)
  Update issue templates
  Delete ISSUE_TEMPLATE.md
  Update issue templates
  Integrate mrdlib redesign (#4361)
  fix "save" button in preference dialog not response (#4406)
  show dialog before creating entry (#4405)
  Scrollbar invisible in Preferences -> BibTex Key Pattern (#4287) (#4398)
  Updates (#4402)
  Fixes for all mods exporter tests (#4396)
  Fix EntryType dialog not closed after generate button (#4390)
  Make rank image narrower (#4395)
  Fix showing multiple icons for one menu entry - fixes #4384 (#4392)
  ...
Siedlerchr added a commit that referenced this pull request Oct 29, 2018
@Siedlerchr
Merge remote-tracking branch 'upstream/master' into docbookfive
4f21871 
* upstream/master: (54 commits)
  Update README.md (#4419)
  Fix generate bibtex key overwrite warning dialog (#4418)
  Fix group hit counter when adding entries (#4413)
  Update README.md
  fix java.nio.file.FileSystemNotFoundException and reorganize code (#4416)
  fixed an issue where corresponding groups are sometimes not highlighted when clicking on entries (#4404)
  Add a few journal abbreviations (#4412)
  Fix display of radio buttons (#4411)
  Update issue templates
  Delete ISSUE_TEMPLATE.md
  Update issue templates
  Integrate mrdlib redesign (#4361)
  fix "save" button in preference dialog not response (#4406)
  show dialog before creating entry (#4405)
  Scrollbar invisible in Preferences -> BibTex Key Pattern (#4287) (#4398)
  Updates (#4402)
  Fixes for all mods exporter tests (#4396)
  Fix EntryType dialog not closed after generate button (#4390)
  Make rank image narrower (#4395)
  Fix showing multiple icons for one menu entry - fixes #4384 (#4392)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Siedlerchr Siedlerchr Siedlerchr approved these changes

@tobiasdiez tobiasdiez tobiasdiez approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@topalavlad @Siedlerchr @tobiasdiez