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

Extended Hints - Alternative to #4971 #4975

Merged
merged 10 commits into from
May 18, 2019
Merged

Conversation

calixtus
Copy link
Member

@calixtus calixtus commented May 17, 2019

fixes #3599 - conflicts with #4971
I realized, talking about changes without presenting a possible solution is just lame.

What do you think?

Sorry about these ugly commit-msgs. I promise improvement.


  • 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 calixtus changed the title Tooltip Preferences - Alternative to #4971 [WIP] Extended Hints - Alternative to #4971 May 17, 2019
@calixtus
Copy link
Member Author

calixtus commented May 17, 2019

Did some rewording according to the suggestion. I'll take a look into the source the next days to identify, what else could be a extended hint.

Suggestions are very welcome.

@davidemdot davidemdot mentioned this pull request May 18, 2019
6 tasks
@calixtus
Copy link
Member Author

calixtus commented May 18, 2019

Sorry about the force-push. I think i still dont have my git-environment set up right...

However, i tried to comletely seperate the logic about whats in the tooltip and about if it's visible or not, and to avoid unnecessary function calls as well. Thanks also to @davidemdot .

About the switchable visibility in the PreferencesDialog: im really not sure if i did this right, because i did not see another proper way to update the visibility. I tested it, it works fine, but is this intended?

@Siedlerchr
Copy link
Member

About the switchable visibility in the PreferencesDialog: im really not sure if i did this right, because i did not see another proper way to update the visibility. I tested it, it works fine, but is this intended?

Not sure what you mean but codewise lgtm so far

@calixtus
Copy link
Member Author

My question is about the call to "updateAfterPreferenceChanges();" in "storeAllSettings();" i put in. UpdateAfter was always called after resetting or importing any preference, but was not after saving and closing. I inserted the call there, but i don't really understand, if this is really the right place. I fear not. ;-)

CHANGELOG.md Outdated Show resolved Hide resolved
@Siedlerchr
Copy link
Member

Ah now I understand what you mean. Not it does not make sense to add it there, cause it will call setValues on each tab -> setting the values for the controls in all preferences tabs. Only useful after you reseted to default, so that all checkboxes etc show default values or after the import of new prefs so that the gui state reflects the prefs state.

When you save you don't need to set the values again, cause you already manually changed the gui states of the controls.

@Siedlerchr
Copy link
Member

Lgtm now! Is this ready now?

@calixtus
Copy link
Member Author

calixtus commented May 18, 2019

Yes, I would say so.
Although there are only two clues that can be hidden now - it is enough for a proof-of-concept.
Further hints can be added together with other pull requests.
Thanks for your feedback.

@calixtus calixtus changed the title [WIP] Extended Hints - Alternative to #4971 Extended Hints - Alternative to #4971 May 18, 2019
@Siedlerchr Siedlerchr added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label May 18, 2019
Copy link
Member

@tobiasdiez tobiasdiez left a comment

Choose a reason for hiding this comment

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

Thanks again!

@tobiasdiez tobiasdiez merged commit accdef3 into JabRef:master May 18, 2019
@calixtus calixtus deleted the tooltip-prefs branch May 18, 2019 18:24
Siedlerchr added a commit that referenced this pull request May 19, 2019
* upstream/master:
  Resize different fonts changing entry type (#4980)
  Extended Hints - Alternative to #4971 (#4975)
  Fix command line help text (#4979)

# Conflicts:
#	src/main/java/org/jabref/gui/search/GlobalSearchBar.java
Siedlerchr added a commit that referenced this pull request May 26, 2019
* 'ieeFix' of github.com:JabRef/jabref:
  Fix the 'Attach file' dialog for starting on the user's main directory (#4996)
  Readme TOC (#4986)
  Fix icon size - the second (#4993)
  Fix background color of dialogs in dark mode (#4994)
  NPE-fix for Preferences/Ext-Prog/Settings for X/Browse  (#4983)
  Bump richtextfx from 0.10.0 to 0.10.1 (#4989)
  Bump tika-core from 1.20 to 1.21 (#4984)
  Resize different fonts changing entry type (#4980)
  Extended Hints - Alternative to #4971 (#4975)
  Fix command line help text (#4979)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Disable extended help in the search field
4 participants