Submenu to select push-application by calixtus · Pull Request #4991 · JabRef/jabref

calixtus · 2019-05-22T09:29:21Z

Since the migration to JavaFX, i saw no way to change the push-application. There were some pull-request in the past, trying to fix this, but they all were forgotten undone. So this is my shot, trying to implement a very simple way to change the push-application.

fixes #674 closes #3942 and closes #4368 closes #5024

~~(Apparently, I forgot the fetch before I opened the new branch. Please ignore the old superfluous commits, my changes start with 95f8668~~

Attention: It's still WIP, it's not done yet.
The caption of the menu is temporary, there is no l10n, Toolbarbutton is not updated and another small Problem is left, i could not figure out yet and I am hoping for your support: I was not able to change the caption of the PushAppplicationMenuItem, since it's bound somehow to another variable. Do you have any Idea, how i can change this label?

----

Change in CHANGELOG.md described
Tests created for changes
Manually tested changed features in running JabRef
Screenshots added in PR description (for bigger UI changes)
Ensured that the git commit message is a good one
Check documentation status (Issue created for outdated help page at help.jabref.org?)

calixtus · 2019-05-22T13:06:49Z

Any suggestions for the label of the submenu? Push-application sounds so technical.

calixtus · 2019-05-22T16:43:48Z

So, theoretically this would be ready to review now...
If I hadn't found a bug in another place.

left is right size after starting JabRef, right is after changing the PushToApp
The button is shown too small after updating. And I know why.

I use the method configureIconButton in the ActionFactory to unbind and refresh the ToolBarButton.
Obviously someone cheated, when he "fixed" an earlier bug with the iconsize in the entry-editor... ;-)

calixtus · 2019-05-22T19:06:53Z

The other Pull Request should fix the button-size-issue. Thats all, im ready to review.

Siedlerchr · 2019-05-22T20:30:38Z

Maybe wording for Push applications
Push to External editor?

Regarding your superfluous commits,if you do a git rebase on the upstream/master (after fetch) then only the new changes should be visible

calixtus · 2019-05-22T21:29:28Z

I think, that didn't work... I stop now rebasing, it's getting out of hand.
The code is the same, i checked twice.

matthiasgeiger · 2019-05-23T03:40:18Z

One question: Which IDE or Git tool are you using?
Special stuff like rebasing is very well supported in IntelliJ (I'm also not capable of rebasing using just the command line ;-))

... passend dazu: https://classicprogrammerpaintings.com/post/142586036029/junior-programmer-learns-git-rebase

calixtus · 2019-05-23T06:06:53Z

It's IntelliJ IDEA CE. I wanted to make sure, if i pass commits, they are formatted right and I read in the documentation, that you recommend this. But I just start to get used to it. In my early days I used eclipse. I pushed with the commandline tool which is probably the cause for this mess.

codecov · 2019-05-27T21:18:28Z

Codecov Report

Merging #4991 into master will decrease coverage by 0.03%.
The diff coverage is 0%.

@@             Coverage Diff              @@
##             master    #4991      +/-   ##
============================================
- Coverage     36.58%   36.54%   -0.04%     
  Complexity     6058     6058              
============================================
  Files          1032     1033       +1     
  Lines         45570    45618      +48     
  Branches       5640     5641       +1     
============================================
  Hits          16670    16670              
- Misses        27600    27648      +48     
  Partials       1300     1300

Impacted Files	Coverage Δ	Complexity Δ
...g/jabref/gui/push/PushToApplicationMenuAction.java	`0% <0%> (ø)`	`0 <0> (?)`
...n/java/org/jabref/gui/preferences/ExternalTab.java	`0% <0%> (ø)`	`0 <0> (ø)`	⬇️
...ain/java/org/jabref/gui/actions/ActionFactory.java	`0% <0%> (ø)`	`0 <0> (ø)`	⬇️
src/main/java/org/jabref/gui/JabRefFrame.java	`0.16% <0%> (-0.01%)`	`1 <0> (ø)`
...a/org/jabref/gui/push/PushToApplicationAction.java	`0% <0%> (ø)`	`0 <0> (ø)`	⬇️
...n/java/org/jabref/gui/actions/StandardActions.java	`0% <0%> (ø)`	`0 <0> (ø)`	⬇️
...org/jabref/gui/push/PushToApplicationsManager.java	`0% <0%> (ø)`	`0 <0> (ø)`	⬇️
...n/java/org/jabref/gui/entryeditor/EntryEditor.java	`0% <0%> (ø)`	`0% <0%> (ø)`	⬇️
...java/org/jabref/gui/filelist/AttachFileAction.java	`0% <0%> (ø)`	`0% <0%> (ø)`	⬇️
... and 4 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0a4210e...236aefb. Read the comment docs.

calixtus · 2019-05-28T13:50:10Z

Cleaned up the commit history, the rebase-monster is finally gone.
As @Siedlerchr pointed out a few days ago: "Nothing git magic can't fix": I sync'd my local master branch with upstream/master, hard-reset my working-branch history to my last real commit, rebased on master and force-pushed to github.

Siedlerchr · 2019-05-28T20:34:17Z

So just one question to help me understand the changes of this a bit better, whenever I change the push application in the tools submenu the icon button toolbar changes to the selected application?

codewise lgtm so far

calixtus · 2019-05-28T20:45:24Z

The RadioMenuItems change

the setting in preferences and
the Action stored in the Manager

and they reconfigure

the MenuItem (the text) and
the ToolBarButton (the icon)

The MenuItem and the ToolBarButton both call the same Action stored in the manager.

tobiasdiez

Thanks for your contribution. Code-wise the changes look good to me and I've only a few a few suggestions.
Although its a huge improvement in terms of usability, I'm not sure if the solution to set the push to application through a separate menu entry is intuitive to most users. I would prefer if the application is set in the preferences instead of via a menu item. @JabRef/developers opinions?

calixtus · 2019-06-02T12:32:57Z

Thanks for your contribution. Code-wise the changes look good to me and I've only a few a few suggestions.
Although its a huge improvement in terms of usability, I'm not sure if the solution to set the push to application through a separate menu entry is intuitive to most users. I would prefer if the application is set in the preferences instead of via a menu item. @JabRef/developers opinions?

Shouldn't be a big deal to put it into the preferences. I'll look into it tonight. I going to create a second pull request for the alternative.

calixtus · 2019-06-02T22:27:08Z

Thanks for your contribution. Code-wise the changes look good to me and I've only a few a few suggestions.
Although its a huge improvement in terms of usability, I'm not sure if the solution to set the push to application through a separate menu entry is intuitive to most users. I would prefer if the application is set in the preferences instead of via a menu item. @JabRef/developers opinions?

I put an alternative proposal in #5024

c8784ff Update and rename emerald-harvard.csl to emerald-harvard-2.csl (#4991) 197ec0e Update circulation-journal.csl (#4994) f36dabe Update American Physical Society dependents (#4992) git-subtree-dir: src/main/resources/csl-styles git-subtree-split: c8784ff

fad76fe Update Gemfile.lock a8dafef Update harvard-cite-them-right.csl (JabRef#4993) c8784ff Update and rename emerald-harvard.csl to emerald-harvard-2.csl (JabRef#4991) 197ec0e Update circulation-journal.csl (JabRef#4994) f36dabe Update American Physical Society dependents (JabRef#4992) 93469bb Create journal-of-musculoskeletal-research.csl (JabRef#4990) 43287c7 Update taxon.csl a little more (JabRef#4989) 9f32917 Update and rename university-college-lillebaelt-harvard.csl to ucl-un… (JabRef#4982) 00c3cec Update isnad-metinici.csl (JabRef#4988) 864a64c Update medicine-and-science-in-sports-and-exercise.csl (JabRef#4983) 0c76011 Don't demote particles in Equine Vet J (JabRef#4984) 603ee73 Fix MLA editor translator (JabRef#4985) 40d207c Update pravnik.csl (JabRef#4986) c5fe30c Update isnad-dipnotlu.csl (JabRef#4987) 469deb6 Sage Harvard: Add translator/editor 1f51ef8 Add Pensoft dependents (JabRef#4981) 5338ff6 Create electrophoresis.csl (JabRef#4883) 4de9a97 Update spec_helper.rb fe81f17 Create uirs-urbani-izziv.csl (JabRef#4928) 69d37d8 Create uirs-urban-challenge-journal.csl (JabRef#4927) d807202 Changes to match Taxon guidelines for authors (JabRef#4980) aeb2170 Create ucl-university-college-denmark-apa.csl (JabRef#4962) 16f58df Update American Physical Society styles (JabRef#4794) b77bd91 Update ucl-university-college-denmark-vancouver.csl (JabRef#4963) 81d1a59 Update bibliotheque-d-archeologie-mediterraneenne-et-africaine-biama.csl (JabRef#4979) 0895562 Create pensoft-journals.csl (JabRef#4972) 4f8fa44 creates the gigascience.csl file (JabRef#4961) 1b9c7a0 Create acta-ornithologica.csl (JabRef#4977) c8c6c6d Create sorbonne-student-law-review.csl (JabRef#4956) git-subtree-dir: src/main/resources/csl-styles git-subtree-split: fad76fe

calixtus commented May 22, 2019

View reviewed changes

Comment thread src/main/java/org/jabref/gui/push/PushToApplicationMenuAction.java Outdated

Siedlerchr reviewed May 22, 2019

View reviewed changes

Comment thread src/main/java/org/jabref/gui/JabRefFrame.java Outdated

calixtus changed the title ~~[WIP] Submenu to select push-application~~ Submenu to select push-application May 22, 2019

This was referenced May 22, 2019

Push to external application JabRef/user-documentation#207

Closed

Fix wrong icon size #4992

Closed

calixtus mentioned this pull request May 22, 2019

Fix icon size - the second #4993

Merged

6 tasks

Siedlerchr added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label May 24, 2019

calixtus added 11 commits May 28, 2019 15:37

Added Submenu to change external push-app

2e6896b

Rewording, Respacing and Optimizing

f420082

Rewording Confusing PushApplications to PushToApplicationsManager

1883f6f

Refactoring and unifying PtAAction with ToolbarButton

d0aa24c

Refactor simplified

6fa926f

Reword l10n

f48a9d4

Refactor for checks

746e025

Added eye-candy

5d6a7cf

Added eye-candy

a8b6489

Changelog

186f9bc

whitespaces

ec5f96b

calixtus force-pushed the menu_select_push_app branch from 236aefb to ec5f96b Compare May 28, 2019 13:41

Siedlerchr approved these changes May 28, 2019

View reviewed changes

calixtus added 4 commits May 30, 2019 22:53

Merge remote-tracking branch 'upstream/master' into menu_select_push_app

91e4393

Rewording, Sorting, Simplifying

2ce9d37

Whitespace

e66586e

Rewording, Sorting

a91455d

tobiasdiez requested changes Jun 2, 2019

View reviewed changes

Comment thread src/main/java/org/jabref/gui/JabRefFrame.java Outdated

Comment thread src/main/java/org/jabref/gui/push/PushToApplicationAction.java Outdated

Refactor var-declaration

eddbe0a

calixtus mentioned this pull request Jun 2, 2019

Select push app in preferences #5024

Merged

6 tasks

calixtus added 2 commits June 3, 2019 01:26

Revert persistent actionInformation

26a1c93

Removed superfluous declaration

2f1637a

tobiasdiez closed this in #5024 Jun 4, 2019

calixtus deleted the menu_select_push_app branch June 4, 2019 22:58

koppor mentioned this pull request Oct 1, 2019

Push to application missing #5375

Closed

koppor added the component: external-application-integration Ingegeration with TeXStudio, Acrobat Reader, ... label Oct 9, 2025

Uh oh!

Conversation

calixtus commented May 22, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

calixtus commented May 22, 2019

Uh oh!

calixtus commented May 22, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

calixtus commented May 22, 2019

Uh oh!

Siedlerchr commented May 22, 2019

Uh oh!

calixtus commented May 22, 2019

Uh oh!

matthiasgeiger commented May 23, 2019

Uh oh!

calixtus commented May 23, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov Bot commented May 27, 2019

Codecov Report

Uh oh!

calixtus commented May 28, 2019

Uh oh!

Siedlerchr commented May 28, 2019

Uh oh!

calixtus commented May 28, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tobiasdiez left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

calixtus commented Jun 2, 2019

Uh oh!

calixtus commented Jun 2, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

calixtus commented May 22, 2019 •

edited

Loading

calixtus commented May 22, 2019 •

edited

Loading

calixtus commented May 23, 2019 •

edited

Loading

calixtus commented May 28, 2019 •

edited

Loading