Added gray background text to authors field to assist newcomers by boceckts · Pull Request #2147 · JabRef/jabref

boceckts · 2016-10-10T09:16:26Z

I added a gray background text to the "Author" and "Editor" field that appears when the fields don't contain any text. See JabRef#61 for reference.

Interal quality assurance
Change in CHANGELOG.md described
Screenshots added (for bigger UI changes)
Manually tested changed features in running JabRef
If you changed the localization: Did you run gradle localizationUpdate?

tschechlovdev

Looks good from my side 👍

tschechlovdev · 2016-10-10T13:15:00Z

+
+        if (!this.hasFocus() && this.getText().isEmpty()) {
+            Font prev = g.getFont();
+            Color prevColor = g.getColor();


Maybe you could rename some variables here.
g- > graphic
h -> height

koppor · 2016-10-11T02:10:00Z

-        super();
-        setupUndoRedo();
-        setupPasteListener();
+        this(null);


Why not "" instead of null?

koppor · 2016-10-11T02:13:52Z

                defaultHeight = 0;
            } else {
-                fieldEditor = new TextArea(field, null);
+                String prompt = (field.equals(FieldName.AUTHOR)) ? Localization.lang("Firstname Lastname and Firstname and Lastname and others") : "";


Wrong text

It should be Firstname Lastname and Firstname Lastname. and others must not be translated as it is a fixed bibtex term used to write et al. or something else as configured in the bibtex style.

The rendering of your example is
Lastname, F.; Firstname; Lastname & others
but it should be
Lastname, F.; Lastname, F. & others

I think, as our "normalize to bibtex name format" formatter will produce "Last, First and Last, First and other", the text should be in this form.

I changed it to Localization.lang("Firstname Lastname and Firstname Lastname") + " and others"

koppor · 2016-10-11T02:16:30Z


-    JTextAreaWithHighlighting(String text) {
-        super(text);
+    public JTextAreaWithHighlighting(String text, String title) {


Please add javadoc comments at least describing title. Isn't title the textWhenNotFocused

Please rename textWhenNotFocused to placeholder to be in line with HTML5 --> http://www.w3schools.com/tags/att_input_placeholder.asp

koppor · 2016-10-11T02:23:49Z

I think, it's difficult to achieve the same effect as html5's placeholders:

There, when clicked in an empty field, the placeholder text remains in light gray. As soon as a key is pressed, the placeholder disappears.

Fun fact: If the focus is put away from JabRef, the text field is displayed as I wish - besides the missing cursor:

Should also be put for the field editor.

Other than that: LGTM, - I assume that before merge, all other required fields also have to get placeholders, This can possibly done by others more familiar with bib(la)tex if the developers agree on this concept.

Siedlerchr · 2016-10-11T07:11:54Z

@koppor This can be achieved with a kind of Custom Component:
http://stackoverflow.com/questions/1738966/java-jtextfield-with-input-hint

matthiasgeiger · 2016-10-11T07:27:22Z

@Siedlerchr thats the way it's currently implemented: Showing/Hiding the hint on FocusGained/FocusLost.

But of course, it is also possible to wait for a "KeyPressedEvent" to delete the placeholder.
However, I think using "isEmpty()" will no longer work, but a flag indicating whether there was an edit (i.e., a key has been pressed) will be necessary.

koppor · 2016-10-11T07:29:27Z

The solution to use WebLookAndFeel (http://stackoverflow.com/a/30782629/873282) also looks promising ( setInptPrompt), but the license of that library is GPL 😟

koppor · 2016-10-11T07:32:40Z

Can't just the simple solution using setText() and getText().equals(promptText) be used? http://stackoverflow.com/a/20200870/873282. Maybe with a combination of "selectAll()" when entering the field so that the users can directly start typing?

boceckts · 2016-10-11T07:35:23Z

Using setText will save the text to the BibTeX source and the text will apear in the fields of the maintable. I guess this is not desired.

On Oct 11, 2016, 09:32, at 09:32, Oliver Kopp notifications@github.com wrote:

Can't just the simple solution using setText() and
getText().equals(promptText) be used?
http://stackoverflow.com/a/20200870/873282

You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub:
#2147 (comment)

matthiasgeiger · 2016-10-11T07:39:24Z

@boceckts Are you sure that a setText(...) will already trigger an edit of the BibEntry? I thought this is performed upon changing the active entry editor textfield...

koppor · 2016-10-11T07:40:06Z

Only when the text field gets focus. 😇

Could you try at the JavaFX branch? The solution http://stackoverflow.com/a/29946224/873282 looks promising.

`.text-field {
    -fx-prompt-text-fill: derive(-fx-control-inner-background,-30%);
}
.text-field:focused {
    -fx-prompt-text-fill: derive(-fx-control-inner-background,-30%);
}`

and add the 'promptText' entry in fxml file:

<TextField fx:id="XY_Id" promptText="First name">

No need to recode the whole tabbing, just a small test and if it works, we can decide to postpone a full implementation until javafx is our main UI technology.

boceckts · 2016-10-11T07:43:03Z

I will give it a try. And yes I think it trigerres when the field loses it's focus.

On Oct 11, 2016, 09:40, at 09:40, Oliver Kopp notifications@github.com wrote:

Only when the text field gets focus. 😇

Could you try at the JavaFX branch? The solution
http://stackoverflow.com/a/29946224/873282 looks promising.

.text-field { -fx-prompt-text-fill: derive(-fx-control-inner-background,-30%); } .text-field:focused { -fx-prompt-text-fill: derive(-fx-control-inner-background,-30%); }

and add the 'promptText' entry in fxml file:

No need to recode the whole tabbing, just a small test and if it works,
we can decide to postpone a full implementation until javafx is our
main UI technology.

You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub:
#2147 (comment)

boceckts · 2016-10-11T11:32:35Z

I changed the behaviour so that the placeholder will always be displayed when the content of the text component is empty. This also means that the "search..." string will also be displayed when nothing is typed even when the search field is focused. I also renamed the classes to JTextAreaWithPlaceholder and JTextFieldWithPlaceholder. I also included the "Editor" field to display the same placeholder.

As @koppor mentioned, JavaFX TextField element has already a palceholder attribute which worked for me as you would expected. (Same as html5)

oscargus · 2016-10-11T18:43:00Z

-                fieldEditor = new TextArea(field, null);
+                String prompt = "";
+                if (field.equals(FieldName.AUTHOR) || field.equals(FieldName.EDITOR)) {
+                     prompt = Localization.lang("Firstname Lastname and Firstname Lastname") + " and others";


I'd suggest removing the and from the localization as well as it is required that it stays and, therefore making it easier to translate.

I changed it accordingly.

# Conflicts: # CHANGELOG.md

tobiasdiez

LGTM

matthiasgeiger · 2016-10-13T13:42:58Z

This is not really working on master, is it?

boceckts · 2016-10-13T13:49:35Z

@matthiasgeiger what do you mean, what exactly does not work? Is there an exception? I tested it on the current masters branch it works as expected...

matthiasgeiger · 2016-10-13T13:54:02Z

Okay... you are right. Sorry, I tried the last build from https://builds.jabref.org/master which are currently outdated 😞

* upstream/master: (24 commits) hotfix NPE in the main table (#2158) Enhance side pane toggle (#1605) Added gray background text to authors field to assist newcomers (#2147) Rewrite google scholar fetcher to new infrastructure (#2101) Found entries will be shown when dropping a pdf with xmp meta data (#2150) Japanese translation (#2155) Add shortcuts to context menu (#2131) [WIP] Doi resolution ignore (#2154) Fix DuplicationChecker and key generator (#2151) just close popup on first ESC keep entry in sight Fix isSharedDatabaseAlreadyPresent check. don't change entry after search if editing an entry (#2129) Change download URL to downloads.jabref.org (#2145) fix switching edited textfield in the entry editor with TAB (#2138) Update me.champeau.gradle.jmh' version from 0.3.0 to 0.3.1 Update install4j plugin from 6.1.2 to 6.1.3 Remove gradle plugin com.github.youribonnaffe.gradle.format as it is deprecated and the config uses a non-existing file Update gradle from 3.0 to 3.1 Fix changing the font size not working when importing preferences (#2069) ... # Conflicts: # src/main/java/net/sf/jabref/gui/externalfiles/DroppedFileHandler.java # src/main/java/net/sf/jabref/gui/importer/fetcher/GoogleScholarFetcher.java

…ef#2147) * Added gray background text to assist newcomers * Display placeholder even when focused * Change placeholder string and include editor field * Update changelog * Fix localization * Refactor localization

boceckts added stupro labels Oct 10, 2016

Added gray background text to assist newcomers

789bf72

boceckts force-pushed the grayBackgroundText branch from 7871c8e to 789bf72 Compare October 10, 2016 10:23

tschechlovdev reviewed Oct 10, 2016

View reviewed changes

tschechlovdev added status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers and removed stupro-ready-for-internal-review labels Oct 10, 2016

tschechlovdev changed the title ~~[WIP] Added gray background text to authors field to assist newcomers~~ Added gray background text to authors field to assist newcomers Oct 10, 2016

koppor reviewed Oct 11, 2016

View reviewed changes

Display placeholder even when focused

6f35ff1

boceckts force-pushed the grayBackgroundText branch from fe9235a to 6f35ff1 Compare October 11, 2016 11:00

Change placeholder string and include editor field

cf6afe5

boceckts force-pushed the grayBackgroundText branch from 0d654b8 to cf6afe5 Compare October 11, 2016 11:16

Update changelog

1dc5249

boceckts added 2 commits October 11, 2016 19:22

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

cf41a69

Fix localization

f6f82d5

oscargus reviewed Oct 11, 2016

View reviewed changes

Refactor localization

31a5612

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

24f2467

# Conflicts: # CHANGELOG.md

tobiasdiez approved these changes Oct 12, 2016

View reviewed changes

tobiasdiez merged commit d962c1a into JabRef:master Oct 12, 2016

boceckts deleted the grayBackgroundText branch October 12, 2016 12:32

boceckts mentioned this pull request Oct 16, 2016

Update vietnam translation #2167

Merged

4 tasks

Uh oh!

Conversation

boceckts commented Oct 10, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tschechlovdev left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

koppor commented Oct 11, 2016

Uh oh!

Siedlerchr commented Oct 11, 2016

Uh oh!

matthiasgeiger commented Oct 11, 2016

Uh oh!

koppor commented Oct 11, 2016

Uh oh!

koppor commented Oct 11, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

boceckts commented Oct 11, 2016

Uh oh!

matthiasgeiger commented Oct 11, 2016

Uh oh!

koppor commented Oct 11, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

boceckts commented Oct 11, 2016

Uh oh!

boceckts commented Oct 11, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tobiasdiez left a comment

Choose a reason for hiding this comment

Uh oh!

matthiasgeiger commented Oct 13, 2016

Uh oh!

boceckts commented Oct 13, 2016

Uh oh!

matthiasgeiger commented Oct 13, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

boceckts commented Oct 10, 2016 •

edited

Loading

koppor commented Oct 11, 2016 •

edited

Loading

koppor commented Oct 11, 2016 •

edited

Loading