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

Entry editor: Add more tooltips and wrap tooltips if necessary #6239

Merged
merged 8 commits into from
Apr 10, 2020

Conversation

systemoperator
Copy link
Contributor

@systemoperator systemoperator commented Apr 3, 2020

@systemoperator
Copy link
Contributor Author

The localization file will be updates as soon as no other changes are requested anymore.

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 for the completion of the fields! I've indeed some comments but nothing big.

@tobiasdiez tobiasdiez added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Apr 4, 2020
@systemoperator
Copy link
Contributor Author

done

@calixtus
Copy link
Member

calixtus commented Apr 5, 2020

Hi @systemoperator,
thanks for your PR.
I was just walking through all PRs to do some housekeeping in github. In some PRs the checkboxes were removed. Even if only one or two of them are applicable to a PR, please keep them, as they are helpful to us to get a quick overview of the extent to which a PR is ready.
I hope you don't mind if I readd them by myself.

@systemoperator
Copy link
Contributor Author

Hi @systemoperator,
thanks for your PR.
I was just walking through all PRs to do some housekeeping in github. In some PRs the checkboxes were removed. Even if only one or two of them are applicable to a PR, please keep them, as they are helpful to us to get a quick overview of the extent to which a PR is ready.
I hope you don't mind if I readd them by myself.

@calixtus Thanks for the information. That's fine. I will keep it in mind. :)

@@ -79,6 +109,8 @@ public String getDescription(Field field) {
case FILE:
case PDF:
return Localization.lang("Link(s) to a local PDF or other document of the work.");
case FOREWORD:
return Localization.lang("Author(s) of a foreword to the work.");
Copy link
Member

Choose a reason for hiding this comment

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

The authors are separated by and? Maybe, this can be added here?

case JOURNALTITLE:
case JOURNAL:
return Localization.lang("Name of a journal, a newspaper, or some other periodical.");
case LABEL:
return Localization.lang("Designation to be used by the citation style as a substitute for the regular label if any data required to generate the regular label is missing.");
case LANGUAGE:
return Localization.lang("Language(s) of the work. Languages may be specified literally or as localisation keys.");
Copy link
Member

Choose a reason for hiding this comment

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

How are multiple languages provided?

case KEYWORDS:
return Localization.lang("Separated list of keywords.");
case RELATED:
return Localization.lang("Citation keys of other entries which have a relationship to this entry.");
Copy link
Member

Choose a reason for hiding this comment

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

Do we need to add "separated by ," here?

Copy link
Member

@koppor koppor left a comment

Choose a reason for hiding this comment

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

I have some minor comments on the text 😇

@tobiasdiez
Copy link
Member

@koppor we agreed the last time that the description should only clarify the semantics of the field and not the bibtex conventions (e.g "and" as separator).

@koppor
Copy link
Member

koppor commented Apr 6, 2020

@koppor we agreed the last time that the description should only clarify the semantics of the field and not the bibtex conventions (e.g "and" as separator).

We should add an ADR here ^^. - Maybe later.

I fully agree for the author thing. I have doubts about the other entries though: What would be important for me is an example for "citation keys" and "languages". The issue is that we use different separators. and for names, , for keywords (even configurable) and (probably) , for these fields.

I really like the Java-by-Comparison approach - and they also favor examples. I would not go so far to make it as long as there:

grafik

However, a quick good example would be really helpful. That would avoid the user to change to a browser and google. Alternatively, the user is forced for a trial-and-error.

@tobiasdiez
Copy link
Member

@koppor for these things, we have the integrity checks right?

I now merge this PR to move forward. Thanks @systemoperator for completing the descriptions!

@tobiasdiez tobiasdiez merged commit bdc5739 into JabRef:master Apr 10, 2020
Siedlerchr added a commit that referenced this pull request Apr 13, 2020
* upstream/master: (23 commits)
  Bump junit-vintage-engine from 5.6.1 to 5.6.2 (#6276)
  Number of items found in ImportEntriesDialog (#6248)
  Fix dependency label
  Bump flexmark-ext-gfm-strikethrough from 0.61.0 to 0.61.6 (#6273)
  Bump unirest-java from 3.7.01 to 3.7.02 (#6275)
  Bump junit-platform-launcher from 1.6.1 to 1.6.2 (#6279)
  Bump flexmark-ext-gfm-tasklist from 0.61.0 to 0.61.6 (#6278)
  Bump junit-jupiter from 5.6.1 to 5.6.2 (#6274)
  Bump flexmark from 0.61.0 to 0.61.6 (#6280)
  Bump classgraph from 4.8.68 to 4.8.69 (#6277)
  Bump org.beryx.jlink from 2.17.4 to 2.17.5 (#6281)
  Improve performance (#6270)
  Improve calculation of matched entries upon change (#6268)
  Fix sort by priority (#6222) (#6265)
  Squashed 'src/main/resources/csl-styles/' changes from 88726cb..db54e56
  Use https everywhere
  Entry editor: Add more tooltips and wrap tooltips if necessary (#6239)
  JDK14 switch - and checkstyle skipping (#6250)
  Bump postgresql from 42.2.11 to 42.2.12 (#6254)
  Bump unirest-java from 3.7.00 to 3.7.01 (#6253)
  ...
Siedlerchr added a commit that referenced this pull request Apr 13, 2020
* upstream/master: (287 commits)
  Bump junit-vintage-engine from 5.6.1 to 5.6.2 (#6276)
  Number of items found in ImportEntriesDialog (#6248)
  Fix dependency label
  Bump flexmark-ext-gfm-strikethrough from 0.61.0 to 0.61.6 (#6273)
  Bump unirest-java from 3.7.01 to 3.7.02 (#6275)
  Bump junit-platform-launcher from 1.6.1 to 1.6.2 (#6279)
  Bump flexmark-ext-gfm-tasklist from 0.61.0 to 0.61.6 (#6278)
  Bump junit-jupiter from 5.6.1 to 5.6.2 (#6274)
  Bump flexmark from 0.61.0 to 0.61.6 (#6280)
  Bump classgraph from 4.8.68 to 4.8.69 (#6277)
  Bump org.beryx.jlink from 2.17.4 to 2.17.5 (#6281)
  Improve performance (#6270)
  Improve calculation of matched entries upon change (#6268)
  Fix sort by priority (#6222) (#6265)
  Squashed 'src/main/resources/csl-styles/' changes from 88726cb..db54e56
  Use https everywhere
  Entry editor: Add more tooltips and wrap tooltips if necessary (#6239)
  JDK14 switch - and checkstyle skipping (#6250)
  Bump postgresql from 42.2.11 to 42.2.12 (#6254)
  Bump unirest-java from 3.7.00 to 3.7.01 (#6253)
  ...
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.

4 participants