Skip to content

Add uiEntryOnFinished() event to uiEntry #290

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

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 54 additions & 3 deletions darwin/entry.m
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,8 @@ - (NSSize)intrinsicContentSize
NSTextField *textfield;
void (*onChanged)(uiEntry *, void *);
void *onChangedData;
void (*onFinished)(uiEntry *, void *);
void *onFinishedData;
};

static BOOL isSearchField(NSTextField *tf)
Expand All @@ -69,6 +71,7 @@ static BOOL isSearchField(NSTextField *tf)
@interface entryDelegateClass : NSObject<NSTextFieldDelegate> {
struct mapTable *entries;
}
- (void)controlTextDidEndEditing:(NSNotification *)note;
- (void)controlTextDidChange:(NSNotification *)note;
- (IBAction)onSearch:(id)sender;
- (void)registerEntry:(uiEntry *)e;
Expand All @@ -91,17 +94,30 @@ - (void)dealloc
[super dealloc];
}

- (void)controlTextDidEndEditing:(NSNotification *)note
{
uiEntry *e;
e = (uiEntry *) mapGet(self->entries, [note object]);
(*(e->onFinished))(e, e->onFinishedData);
}


- (void)controlTextDidChange:(NSNotification *)note
{
[self onSearch:[note object]];
uiEntry *e;
e = (uiEntry *) mapGet(self->entries, [note object]);
(*(e->onChanged))(e, e->onChangedData);
}

- (IBAction)onSearch:(id)sender
{
uiEntry *e;

e = (uiEntry *) mapGet(self->entries, sender);

NSSearchField *s;
s = (NSSearchField *) (e->textfield);
(*(e->onChanged))(e, e->onChangedData);
(*(e->onFinished))(e, e->onFinishedData);
}

- (void)registerEntry:(uiEntry *)e
Expand Down Expand Up @@ -155,6 +171,31 @@ void uiEntryOnChanged(uiEntry *e, void (*f)(uiEntry *, void *), void *data)
e->onChangedData = data;
}

// NOTE: for search widgets on OSX, setting OnFinished() alters the behaviour
// (by setting sendsWholeSearchString).
// Standard text and password entry widgets are not affected.
// background:
// On OSX, there doesn't seem to be any simple way to catch
// both 'changed' events and 'enter' (finished editing) events on
// search widgets. Instead, there just a single 'search' event, and flags
// to determine when it triggers. By default, it triggers after each keypress
// (with a little delay in case the user is still typing).
// There's also an option to change the behaviour to trigger only when the
// enter key is hit, or the search icon is pressed. This is the
// sendsWholeSearchString flag, and we'll set it if (and only if) OnFinished()
// is used.
Copy link
Owner

Choose a reason for hiding this comment

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

Good comment here! As for the text itself, yes, that is an issue, and I'm not sure what workarounds there are. The control editing system in OS X is something I don't fully understand yet, especially since NSSearchField can also use target-action. I'll have to look into it more...

void uiEntryOnFinished(uiEntry *e, void (*f)(uiEntry *, void *), void *data)
{
e->onFinished = f;
e->onFinishedData = data;
if (isSearchField(e->textfield)) {
NSSearchField *s;
s = (NSSearchField *) (e->textfield);
// TODO requires OSX >= 10.10 (is that an issue?)
Copy link
Owner

Choose a reason for hiding this comment

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

Calling it directly is, but this kind of thing is what future.m is for.

Copy link
Owner

Choose a reason for hiding this comment

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

Actually the equivalent property on NSSearchFieldCell is 10.3 and newer, so just replace s in the call with [s cell] (or preferably, given the coding style I've written myself into with this and that I should probably clan up someday, a cast version of [s cell]).

[s setSendsWholeSearchString:YES];
}
}

int uiEntryReadOnly(uiEntry *e)
{
return [e->textfield isEditable] == NO;
Expand All @@ -175,6 +216,11 @@ static void defaultOnChanged(uiEntry *e, void *data)
// do nothing
}

static void defaultOnFinished(uiEntry *e, void *data)
{
// do nothing
}

// these are based on interface builder defaults; my comments in the old code weren't very good so I don't really know what talked about what, sorry :/
void finishNewTextField(NSTextField *t, BOOL isEntry)
{
Expand Down Expand Up @@ -219,8 +265,13 @@ void finishNewTextField(NSTextField *t, BOOL isEntry)
[delegates addObject:entryDelegate];
}
[entryDelegate registerEntry:e];
uiEntryOnChanged(e, defaultOnChanged, NULL);

// set the callbacks directly, so as to not trigger the
// sendsWholeSearchString flag set in OnFinished() for search widgets.
e->onFinished = defaultOnFinished;
e->onFinishedData = NULL;
e->onChanged = defaultOnChanged;
e->onChangedData = NULL;
return e;
}

Expand Down
33 changes: 30 additions & 3 deletions examples/controlgallery/main.c
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,27 @@ static int onShouldQuit(void *data)
return 1;
}


static void onEntryChanged(uiEntry* e, void* data)
{
const char* msg = data;
printf("%s: OnChanged\n", msg);
}

static void onEntryFinished(uiEntry* e, void* data)
{
const char* msg = data;
printf("%s: OnFinished\n", msg);
}


static uiControl *makeBasicControlsPage(void)
{
uiBox *vbox;
uiBox *hbox;
uiGroup *group;
uiForm *entryForm;
uiEntry *e;

vbox = uiNewVerticalBox();
uiBoxSetPadded(vbox, 1);
Expand Down Expand Up @@ -54,18 +69,30 @@ static uiControl *makeBasicControlsPage(void)
uiFormSetPadded(entryForm, 1);
uiGroupSetChild(group, uiControl(entryForm));

e = uiNewEntry();
uiEntryOnChanged(e, onEntryChanged, "entry");
uiEntryOnFinished(e, onEntryFinished, "entry");
uiFormAppend(entryForm,
"Entry",
uiControl(uiNewEntry()),
uiControl(e),
0);

e = uiNewPasswordEntry();
uiEntryOnChanged(e, onEntryChanged, "password");
uiEntryOnFinished(e, onEntryFinished, "password");
uiFormAppend(entryForm,
"Password Entry",
uiControl(uiNewPasswordEntry()),
uiControl(e),
0);

e = uiNewSearchEntry();
uiEntryOnChanged(e, onEntryChanged, "search");
uiEntryOnFinished(e, onEntryFinished, "search");
uiFormAppend(entryForm,
"Search Entry",
uiControl(uiNewSearchEntry()),
uiControl(e),
0);

uiFormAppend(entryForm,
"Multiline Entry",
uiControl(uiNewMultilineEntry()),
Expand Down
1 change: 1 addition & 0 deletions ui.h
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,7 @@ typedef struct uiEntry uiEntry;
_UI_EXTERN char *uiEntryText(uiEntry *e);
_UI_EXTERN void uiEntrySetText(uiEntry *e, const char *text);
_UI_EXTERN void uiEntryOnChanged(uiEntry *e, void (*f)(uiEntry *e, void *data), void *data);
_UI_EXTERN void uiEntryOnFinished(uiEntry *e, void (*f)(uiEntry *e, void *data), void *data);
_UI_EXTERN int uiEntryReadOnly(uiEntry *e);
_UI_EXTERN void uiEntrySetReadOnly(uiEntry *e, int readonly);
_UI_EXTERN uiEntry *uiNewEntry(void);
Expand Down
24 changes: 23 additions & 1 deletion unix/entry.c
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,15 @@ struct uiEntry {
void (*onChanged)(uiEntry *, void *);
void *onChangedData;
gulong onChangedSignal;
void (*onFinished)(uiEntry *, void *);
void *onFinishedData;
};

uiUnixControlAllDefaults(uiEntry)

static void onChanged(GtkEditable *editable, gpointer data)
{
uiEntry *e = uiEntry(data);

(*(e->onChanged))(e, e->onChangedData);
}

Expand All @@ -25,6 +26,17 @@ static void defaultOnChanged(uiEntry *e, void *data)
// do nothing
}

static void onFinished(GtkEditable *editable, gpointer data)
{
uiEntry *e = uiEntry(data);
(*(e->onFinished))(e, e->onFinishedData);
}

static void defaultOnFinished(uiEntry *e, void *data)
{
// do nothing
}

char *uiEntryText(uiEntry *e)
{
return uiUnixStrdupText(gtk_entry_get_text(e->entry));
Expand All @@ -45,6 +57,13 @@ void uiEntryOnChanged(uiEntry *e, void (*f)(uiEntry *, void *), void *data)
e->onChangedData = data;
}

void uiEntryOnFinished(uiEntry *e, void (*f)(uiEntry *, void *), void *data)
{
e->onFinished = f;
e->onFinishedData = data;
}


int uiEntryReadOnly(uiEntry *e)
{
return gtk_editable_get_editable(e->editable) == FALSE;
Expand Down Expand Up @@ -73,6 +92,9 @@ static uiEntry *finishNewEntry(GtkWidget *w, const gchar *signal)
e->onChangedSignal = g_signal_connect(e->widget, signal, G_CALLBACK(onChanged), e);
uiEntryOnChanged(e, defaultOnChanged, NULL);

g_signal_connect(e->widget, "activate", G_CALLBACK(onFinished), e);
Copy link
Owner

@andlabs andlabs Jan 15, 2018

Choose a reason for hiding this comment

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

Would editing-done (in GtkCellEditable) be a better signal here? The docs seem to imply they are connected, but... (TODO to self: check the source code)

Note to self: TODO investigate the phrasing "it is also commonly used by applications to intercept activation of entries" in the docs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'd discounted editing-done because GtkCellEditable is designed for entry widgets embedded in tables rather than stand-alone. But the docs do rather imply that stock GtkEntry always supports the GtkCellEditable interface.
So I had another go at it, but no dice. Just couldn't see any editing-done signals triggering. I might have been doing it all wrong, but a internet search seems strangely silent on the whole subject.
Like you said, time to dig through the Gtk source code.

I see the "it is also commonly used by applications to intercept activation of entries" line as acknowledging that the activate signal is used for this purpose by a lot of apps, even if it wasn't originally intended for this use. And, given that, I reckon activate is a sound enough method - it works and is unlikely to be removed any time soon.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ahh - I've just been poking through the Gtk+ source code.
Calling gtk_cell_editable_start_editing () on GtkEntry will cause it to wire up it's activate signal to a callback that emits both the editing-done and remove-widget GtkCellEditable signals.
(see the GtkCellEditable implementation for GtkEntry: https://git.gnome.org/browse/gtk+/tree/gtk/gtkentry.c#n4587 )

So I think we're better off sticking with activate.

uiEntryOnFinished(e, defaultOnFinished, NULL);

return e;
}

Expand Down
34 changes: 27 additions & 7 deletions windows/entry.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,19 +7,27 @@ struct uiEntry {
void (*onChanged)(uiEntry *, void *);
void *onChangedData;
BOOL inhibitChanged;
void (*onFinished)(uiEntry *, void *);
void *onFinishedData;
};

static BOOL onWM_COMMAND(uiControl *c, HWND hwnd, WORD code, LRESULT *lResult)
{
uiEntry *e = uiEntry(c);

if (code != EN_CHANGE)
return FALSE;
if (e->inhibitChanged)
return FALSE;
(*(e->onChanged))(e, e->onChangedData);
*lResult = 0;
return TRUE;
if (code == EN_CHANGE) {
if (e->inhibitChanged)
return FALSE;
(*(e->onChanged))(e, e->onChangedData);
*lResult = 0;
return TRUE;
}
if (code == EN_KILLFOCUS) {
Copy link
Owner

@andlabs andlabs Jan 15, 2018

Choose a reason for hiding this comment

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

Windows was the thing I was the least sure about what constitutes "finished editing", because the traditional wisdom was (as far as I can gather) to validate everything when dismissing a dialog box. My original idea for validating correctness of user entries was to provide a function uiEntryAlertInvalid() that would display a notification that didn't interrupt entry (on Windows, it'd use EM_SHOWBALLOONTIP) and you'd do this on the Changed event, but I'm not sure what people would prefer doing :/ (Pre-libui package ui actually did have this, but I've now forgotten what I called the method back then.)

That being said, I'd suggest keeping these in mind:

https://blogs.msdn.microsoft.com/oldnewthing/20040419-00/?p=39753
https://blogs.msdn.microsoft.com/oldnewthing/20050808-16/?p=34673 (though this seems to imply that EN_KILLFOCUS doesn't have the same pitfalls that WM_KILLFOCUS does, thus invalidating this whole comment? TODO...)
https://blogs.msdn.microsoft.com/oldnewthing/20090420-00/?p=18523

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Isn't dialog validation a slightly moot point anyway, without dialog box support?
I'd guess that a hypothetical dialog box would have some OnDismiss() callback that's invoked when OK or cancel is pressed. And that's where dialog-validation-as-a-whole code would go. I don't think that handling EN_KILLFOCUS here would preclude hooking into whatever windows (and gtk and osx).

Caveat: I have no experience or knowledge whatsoever about dialog boxes on any of the target platforms :-)

The use case I'm interested in myself is less to do with validation, and more to do with reacting to a "completed" change, rather than the higher-frequency OnChanged() callbacks. In particular, I have a search box where the searches take a few seconds to perform, so I only want to perform them when the user has completed entering their search query.

Copy link
Contributor Author

@bcampbell bcampbell Jan 16, 2018

Choose a reason for hiding this comment

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

And yes, my reading of those Raymond Chen posts also seems to suggest that WM_KILLFOCUS is the tricky one, rather than EN_FOCUS.

(*(e->onFinished))(e, e->onFinishedData);
*lResult = 0;
return TRUE;
}
return FALSE;
}

static void uiEntryDestroy(uiControl *c)
Expand Down Expand Up @@ -56,6 +64,11 @@ static void defaultOnChanged(uiEntry *e, void *data)
// do nothing
}

static void defaultOnFinished(uiEntry *e, void *data)
{
// do nothing
}

char *uiEntryText(uiEntry *e)
{
return uiWindowsWindowText(e->hwnd);
Expand All @@ -76,6 +89,12 @@ void uiEntryOnChanged(uiEntry *e, void (*f)(uiEntry *, void *), void *data)
e->onChangedData = data;
}

void uiEntryOnFinished(uiEntry *e, void (*f)(uiEntry *, void *), void *data)
{
e->onFinished = f;
e->onFinishedData = data;
}

int uiEntryReadOnly(uiEntry *e)
{
return (getStyle(e->hwnd) & ES_READONLY) != 0;
Expand Down Expand Up @@ -106,6 +125,7 @@ static uiEntry *finishNewEntry(DWORD style)

uiWindowsRegisterWM_COMMANDHandler(e->hwnd, onWM_COMMAND, uiControl(e));
uiEntryOnChanged(e, defaultOnChanged, NULL);
uiEntryOnFinished(e, defaultOnFinished, NULL);

return e;
}
Expand Down