Why is centralWidget not accepting shortcuts anymore?

cerr

Hi,

First off, I'm a Qt newbie...
So, I'm working on a a Qt application which is using QTabWidgets, QGroubBoxes, QSplitters and a Konsole terminal window and looks something like this:

Okay, you see that GroupBox titled "Test" there? I've added it around the terminal window to - down the road, be able to group things together. Now, there's a function addTerminal() which gets invoked, when a new terminal is added. I've manipulated it so, that I create a QGroupBox first and add the new terminal within it. Done! But Now, I realize that my centralWidget doesn't seem to be accepting the shortcuts anymore. Why not? Does the groupbox (and what's within it)not belong to the central widget anymore?
The pointer to the centralWidget gets extracted like:

MainWindow::MainWindow(QWidget *parent) : KMainWindow(parent) {

    const char *app_name = "kterminal";
    VersionStr = "0.0.1";
    this->setWindowTitle(QApplication::translate(app_name, app_name, 0));
    if (this->objectName().isEmpty()) {
        this->setObjectName(app_name);
    }

    centralWidget = new QWidget(this);
    centralWidget->setObjectName(QStringLiteral("centralWidget"));

    m_sessionStack = new SessionStack(centralWidget, this);
    m_sessionStack->addSession();

I'm not exactly sure how to redirect the QShortcuts if they do not work by sending them to m_sessionStack. Any assistance, hints or tips would be appreciated!

Chris Kawa

I'm assuming the terminal is not read-only? Input widgets (like QLineEdit, QTextEdit etc.) intentionally block shortcuts when they get focus to prevent triggering them when user is typing inside them. You can override the keyPressEvent and trigger your shortcuts there if you want to.

cerr

@Chris-Kawa
I'm not sure if that is the problem because it worked fine before I added the QGroupBox

Chris Kawa

Can you show the code that adds the groupbox?

cerr

@Chris-Kawa said in Why is centralWidget not accepting shortcuts anymore?:

Can you show the code that adds the groupbox?

Yep, that is:

Terminal* Session::addTerminal(QWidget* parent)
{    
  
    QGroupBox *groupBox = new QGroupBox(tr("Test"), parent);    
    groupBox->setAlignment(Qt::AlignLeft);
    QVBoxLayout *vbox = new QVBoxLayout;    
    groupBox->setLayout(vbox);
    vbox->addWidget(groupBox);
    
    Terminal* terminal = new Terminal(parent);
    connect(terminal, SIGNAL(activated(int)), this, SLOT(setActiveTerminal(int)));
    connect(terminal, SIGNAL(manuallyActivated(Terminal*)), this, SIGNAL(terminalManuallyActivated(Terminal*)));
    connect(terminal, SIGNAL(titleChanged(int,QString)), this, SLOT(setTitle(int,QString)));
    connect(terminal, SIGNAL(keyboardInputBlocked(Terminal*)), this, SIGNAL(keyboardInputBlocked(Terminal*)));
    connect(terminal, SIGNAL(silenceDetected(Terminal*)), this, SIGNAL(silenceDetected(Terminal*)));
    connect(terminal, SIGNAL(destroyed(int)), this, SLOT(cleanup(int)));
    m_terminals.insert(terminal->id(), terminal);    
    QWidget* terminalWidget = terminal->terminalWidget();
    
    vbox->addWidget(terminalWidget);
    
    terminalWidget->show();
    if (groupBox) groupBox->setFocus();
    return terminal;
}

and with working shortcuts, this looked like:

Terminal* Session::addTerminal(QWidget* parent)
{
    Terminal* terminal = new Terminal(parent);
    connect(terminal, SIGNAL(activated(int)), this, SLOT(setActiveTerminal(int)));
    connect(terminal, SIGNAL(manuallyActivated(Terminal*)), this, SIGNAL(terminalManuallyActivated(Terminal*)));
    connect(terminal, SIGNAL(titleChanged(int,QString)), this, SLOT(setTitle(int,QString)));
    connect(terminal, SIGNAL(keyboardInputBlocked(Terminal*)), this, SIGNAL(keyboardInputBlocked(Terminal*)));
    connect(terminal, SIGNAL(silenceDetected(Terminal*)), this, SIGNAL(silenceDetected(Terminal*)));
    connect(terminal, SIGNAL(destroyed(int)), this, SLOT(cleanup(int)));

    m_terminals.insert(terminal->id(), terminal);

    QWidget* terminalWidget = terminal->terminalWidget();
    if (terminalWidget) terminalWidget->setFocus();

    return terminal;
}

Chris Kawa

This part is wrong:

groupBox->setLayout(vbox);
vbox->addWidget(groupBox);

It adds a layout to a widget and then that widget to that layout. It's kinda like the tail-eating snake ;)

Also you should probably put that terminal (or a groupbox) into a layout of the parent, not just create it with a parent i.e.

Terminal* Session::addTerminal(QWidget* parent)
{    
    Terminal* terminal = new Terminal(parent);
    connect(terminal, SIGNAL(activated(int)), this, SLOT(setActiveTerminal(int)));
    connect(terminal, SIGNAL(manuallyActivated(Terminal*)), this, SIGNAL(terminalManuallyActivated(Terminal*)));
    connect(terminal, SIGNAL(titleChanged(int,QString)), this, SLOT(setTitle(int,QString)));
    connect(terminal, SIGNAL(keyboardInputBlocked(Terminal*)), this, SIGNAL(keyboardInputBlocked(Terminal*)));
    connect(terminal, SIGNAL(silenceDetected(Terminal*)), this, SIGNAL(silenceDetected(Terminal*)));
    connect(terminal, SIGNAL(destroyed(int)), this, SLOT(cleanup(int)));
    m_terminals.insert(terminal->id(), terminal);    

    // get parent layout. if parent has no layout create one for it
    auto parent_layout = parent->layout();
    if (!parent_layout) {
       parent_layout = new QVboxLayout();
       parent->setLayout(parent_layout); 
    }

    // create a groupbox and give it a layout
    QGroupBox *groupBox = new QGroupBox(tr("Test"));
    groupBox->setAlignment(Qt::AlignLeft);
    groupBox->setLayout(new QVBoxLayout());
    
    // put the terminal widget into groupBox's layout
    groupBox->layout()->addWidget(terminal->terminalWidget()); 

   //put the groupBox into parent's layout
   parent_layout->addWidget(groupBox);
    
    // groupbox is not an input widget. It doesn't take focus
    //if (groupBox) groupBox->setFocus();

    return terminal;
}

Important to note is that when you put a widget into a layout it is re-parented to the widget governed by that layout, so avoid code like this:

QWidget* w = new QWidget(parent); //sets parent
QLayout* lay = new QVBoxLayout(parent); //sets layout on a parent
parent->setLayout(lay); //sets the same layout again and you'll get a runtime warning
lay->addWidget(w); //sets the same parent again

You can omit the parameter to the constructors of the widget and layout:

QWidget* w = new QWidget();
QLayout* lay = new QVBoxLayout();
parent->setLayout(lay); //now sets the layout once, the widget takes ownership of it
lay->addWidget(w); //now puts the w in a layout and gives it a parent

cerr

@Chris-Kawa

Alright,

Thanks for that!
I however had to insert a terminal->terminalWidget()->show(); after the addWidget() in order to get the terminal to show within the GroupBox. But I'm seemingly still at the same spot, the keyboard shortcuts do not go anywhere. My shortcut connects in mainwindow.cpp look like:

void MainWindow::createShortcuts()
{
      m_left_tab_shortcut = new QShortcut(QKeySequence("Shift+Left"), this);
    QObject::connect(m_left_tab_shortcut, SIGNAL(activated()),
                     m_sessionStack, SLOT(select_left_tab()));

    m_right_tab_shortcut = new QShortcut(QKeySequence("Shift+Right"), this);
    QObject::connect(m_right_tab_shortcut, SIGNAL(activated()),
                     m_sessionStack, SLOT(select_right_tab()));

    m_split_horizontal_shortcut = new QShortcut(QKeySequence("Ctrl+'"), this);
    QObject::connect(m_split_horizontal_shortcut, SIGNAL(activated()),
                     m_sessionStack, SLOT(horizontal_split_current_terminal()));

    m_split_vertical_shortcut = new QShortcut(QKeySequence("Ctrl+;"), this);
    QObject::connect(m_split_vertical_shortcut, SIGNAL(activated()),
                     m_sessionStack, SLOT(vertical_split_current_terminal()));

    QShortcut *m_new_tab_shortcut = new QShortcut(QKeySequence("Ctrl+t"), this);
    QObject::connect(m_new_tab_shortcut, SIGNAL(activated()), m_sessionStack, SLOT(addSession()));
}

cerr

@Chris-Kawa said in Why is centralWidget not accepting shortcuts anymore?:

This part is wrong:

groupBox->setLayout(vbox);
vbox->addWidget(groupBox);

It adds a layout to a widget and then that widget to that layout. It's kinda like the tail-eating snake ;)

Also you should probably put that terminal (or a groupbox) into a layout of the parent, not just create it with a parent i.e.

Terminal* Session::addTerminal(QWidget* parent)
{    
    Terminal* terminal = new Terminal(parent);
    connect(terminal, SIGNAL(activated(int)), this, SLOT(setActiveTerminal(int)));
    connect(terminal, SIGNAL(manuallyActivated(Terminal*)), this, SIGNAL(terminalManuallyActivated(Terminal*)));
    connect(terminal, SIGNAL(titleChanged(int,QString)), this, SLOT(setTitle(int,QString)));
    connect(terminal, SIGNAL(keyboardInputBlocked(Terminal*)), this, SIGNAL(keyboardInputBlocked(Terminal*)));
    connect(terminal, SIGNAL(silenceDetected(Terminal*)), this, SIGNAL(silenceDetected(Terminal*)));
    connect(terminal, SIGNAL(destroyed(int)), this, SLOT(cleanup(int)));
    m_terminals.insert(terminal->id(), terminal);    

    // get parent layout. if parent has no layout create one for it
    auto parent_layout = parent->layout();
    if (!parent_layout) {
       parent_layout = new QVboxLayout();
       parent->setLayout(parent_layout); 
    }

    // create a groupbox and give it a layout
    QGroupBox *groupBox = new QGroupBox(tr("Test"));
    groupBox->setAlignment(Qt::AlignLeft);
    groupBox->setLayout(new QVBoxLayout());
    
    // put the terminal widget into groupBox's layout
    groupBox->layout()->addWidget(terminal->terminalWidget()); 

   //put the groupBox into parent's layout
   parent_layout->addWidget(groupBox);
    
    // groupbox is not an input widget. It doesn't take focus
    //if (groupBox) groupBox->setFocus();

    return terminal;
}

Important to note is that when you put a widget into a layout it is re-parented to the widget governed by that layout, so avoid code like this:

QWidget* w = new QWidget(parent); //sets parent
QLayout* lay = new QVBoxLayout(parent); //sets layout on a parent
parent->setLayout(lay); //sets the same layout again and you'll get a runtime warning
lay->addWidget(w); //sets the same parent again

You can omit the parameter to the constructors of the widget and layout:

QWidget* w = new QWidget();
QLayout* lay = new QVBoxLayout();
parent->setLayout(lay); //now sets the layout once, the widget takes ownership of it
lay->addWidget(w); //now puts the w in a layout and gives it a parent

Hold on, I was a bit too quick up there! I now get this message when my application starts up (using the above code):
Adding a QLayout to a QSplitter is not supported.
Since my parent is a QSplitter - how do I solve this alrenatively? Do I need to have some kind of intermittent Widget that can be split and supports QLayout, too?
I tried to remove the parent_layout and instead use parent->layout()->addWidget(groupBox); but that doesn't work either, I get *** Crashed with return code: 0 ***

Chris Kawa

@cerr said:

Since my parent is a QSplitter

If that's the case you can skip the parent_layout part entirely. Change your function signature to:

Terminal* Session::addTerminal(QSplitter* parent)

and add the groupbox directly to the splitter:

parent->addWidget(groupBox);

The design of your app seems a little lacking though, since you have mixed concerns in a single class. For example why would session know anything about terminals or widgets. It shouldn't be concerned with splitters, layouts etc. It's a session, non-ui entity.
I'd go for something like this instead:

// non -ui part:
Session* session = new Session(...) //create a session
Terminal* terminal = new Terminal(session, ...); // create a terminal for that session, whatever the heck a terminal is :)

//ui part:
MainWindow* main_window = new MainWindow();
TerminalWidget* = term_widget = new TerminalWidget(terminal); //create a ui element for the terminal (i.e the groupbox with contents)
main_window->addWidget(term_widget); //inserts the terminal into the splitter

This way non-ui elements don't have to know about widgets, layouts etc, main window doesn't have to know what a session, terminal or terminal widget is and the terminal widget doesn't have to distinguish if it is being inserted into a layout or a splitter or anything else. Each component does only its own thing and you get clean separation of duties.

Going back to your original shortcut problem - nothing obvious stands out to me. Could you extract the relevant parts of your app into some small reproducible pack and upload it somewhere so we could have a look?

cerr

@Chris-Kawa

Going back to your original shortcut problem - nothing obvious stands out to me. Could you extract the relevant parts of your app into some small reproducible pack and upload it somewhere so we could have a look?

I have tried around but can't get it going and it's kinda difficult to squeeze it down but if you want to have a look at the full thing, the sources are available on https://github.com/reggler/kterminal

Thanks!