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

EditBox: Only emit onReturnOrUnfocus once in cases where Return is pressed and the widget loses focus because of it #227

Merged

Conversation

CasualYT31
Copy link
Contributor

With the following code:

#include <TGUI/TGUI.hpp>
#include <TGUI/Backend/SFML-Graphics.hpp>
#include <iostream>

using namespace tgui;

int main() {
    sf::RenderWindow window{ {800, 600}, "TGUI example - SFML_GRAPHICS backend" };
    Gui gui{ window };

    const auto t = TabContainer::create();
    gui.add(t);

    const auto p = t->addTab("Test");
    t->addTab("AnotherTab", false);

    const auto edit = EditBox::create();
    p->add(edit);

    edit->onReturnOrUnfocus([&]() {
        std::cout << "Calling\n";
        t->select(1);
    });

    while (window.isOpen()) {
        sf::Event event;
        while (window.pollEvent(event)) {
            gui.handleEvent(event);

            if (event.type == sf::Event::Closed)
                window.close();
        }
        window.clear();
        gui.draw();
        window.display();
    }
}

If you click off the EditBox onto the Panel it's within, you will see "Calling" output once. However, if you press Return, you will see "Calling" output twice: once for the Return key press, and another for when the EditBox loses focus due to the Tab selection.

Incidentally, if you run the above code without the patch merged to master recently, a stack overflow will occur since the EditBox will never lose focus due to the Widget::setFocus() call being invoked last and not first.

Although the signal being invoked twice is technically correct, I feel like it goes against the spirit of the signal. If you use this signal, you'd typically only want to react to one of the Return key press and the unfocus at a time. So this PR adds a small piece of code that prevents emitting the signal more than once at a time.

I understand if this change is rejected, although I would argue if you wanted to react to both of them at the same time you'd just connect to the onUnfocus and onReturnKeyPress signals explicitly.

@texus
Copy link
Owner

texus commented Dec 19, 2023

It's technically correct for it to be executed twice, but there is little point in it being triggered twice. So I'm willing to accept the PR.

Could you swap the places of the declarations of m_onReturnOrUnfocusEmitted and emitReturnOrUnfocus though? TGUI always places the member functions first (in order of public, protected and then private) and then afterwards the member variables (again with public first and private later). Right now m_onReturnOrUnfocusEmitted is declared in the section for functions while the emitReturnOrUnfocus function is placed in the section for variables.

I would also change "Used to prevent emitting onReturnOrUnfocus more than once in certain cases" to something like
"Used to prevent emitting onReturnOrUnfocus twice when unfocusing the edit box inside the callback function" to be more specific about why the variable exists.

If you can make those changes then I'll merge the PR.

@CasualYT31 CasualYT31 force-pushed the editbox-only-emit-returnorunfocused-once branch from e6d0066 to ee41246 Compare December 20, 2023 08:49
…essed and the widget loses focus because of it
@CasualYT31 CasualYT31 force-pushed the editbox-only-emit-returnorunfocused-once branch from ee41246 to fcfc74e Compare December 20, 2023 08:51
@texus texus merged commit 59bc2ae into texus:1.x Dec 20, 2023
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants