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

Search bar: improve keypress handling, fix glitch in popup, strip whitespaces #4658

Merged
merged 9 commits into from
May 6, 2022
76 changes: 50 additions & 26 deletions src/widget/wsearchlineedit.cpp
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#include "wsearchlineedit.h"

#include <QAbstractItemView>
#include <QApplication>
#include <QFont>
#include <QLineEdit>
#include <QShortcut>
Expand Down Expand Up @@ -125,15 +126,6 @@ WSearchLineEdit::WSearchLineEdit(QWidget* pParent, UserSettingsPointer pConfig)
this,
&WSearchLineEdit::slotIndexChanged);

// When you hit enter, it will trigger or clear the search.
connect(this->lineEdit(),
&QLineEdit::returnPressed,
this,
[this] {
if (!slotClearSearchIfClearButtonHasFocus()) {
slotTriggerSearch();
}
});
loadQueriesFromConfig();

refreshState();
Expand Down Expand Up @@ -233,7 +225,7 @@ void WSearchLineEdit::loadQueriesFromConfig() {
m_pConfig->getKeysWithGroup(kSavedQueriesConfigGroup);
QSet<QString> queryStrings;
for (const auto& queryKey : queryKeys) {
const auto& queryString = m_pConfig->getValueString(queryKey);
QString queryString = m_pConfig->getValueString(queryKey).trimmed();
if (queryString.isEmpty() || queryStrings.contains(queryString)) {
// Don't add duplicate and remove it from the config immediately
m_pConfig->remove(queryKey);
Expand All @@ -259,7 +251,7 @@ void WSearchLineEdit::saveQueriesInConfig() {
for (int index = 0; index < count(); index++) {
m_pConfig->setValue(
ConfigKey(kSavedQueriesConfigGroup, QString::number(index)),
itemText(index));
itemText(index).trimmed());
}
}

Expand Down Expand Up @@ -351,6 +343,10 @@ void WSearchLineEdit::keyPressEvent(QKeyEvent* keyEvent) {
}
break;
case Qt::Key_Enter:
case Qt::Key_Return:
if (slotClearSearchIfClearButtonHasFocus()) {
return;
}
if (findCurrentTextIndex() == -1) {
slotSaveSearch();
}
Expand All @@ -366,11 +362,22 @@ void WSearchLineEdit::keyPressEvent(QKeyEvent* keyEvent) {
}
return;
}
// Space in popup emulates Return press
if (view()->isVisible()) {
QKeyEvent returnPress(QEvent::KeyPress, Qt::Key_Return, Qt::NoModifier);
QApplication::sendEvent(view(), &returnPress);
return;
}
break;
case Qt::Key_Escape:
emit setLibraryFocus(FocusWidget::TracksTable);
return;
default:
// Don't change the query while the popup is open. This would cause the
// same weird vertical squeezing like removing the current index.
if (view()->isVisible()) {
return;
}
break;
}

Expand All @@ -383,6 +390,7 @@ void WSearchLineEdit::focusInEvent(QFocusEvent* event) {
<< "focusInEvent";
#endif // ENABLE_TRACE_LOG
QComboBox::focusInEvent(event);
updateClearAndDropdownButton(currentText());
}

void WSearchLineEdit::focusOutEvent(QFocusEvent* event) {
Expand Down Expand Up @@ -463,27 +471,31 @@ void WSearchLineEdit::slotTriggerSearch() {

/// saves the current query as selection
void WSearchLineEdit::slotSaveSearch() {
m_saveTimer.stop();
QString cText = currentText().trimmed();
int cIndex = findCurrentTextIndex();
uklotzde marked this conversation as resolved.
Show resolved Hide resolved
#if ENABLE_TRACE_LOG
kLogger.trace()
<< "save search. Index: "
<< "save search. Text:"
<< cText
<< "Index:"
<< cIndex;
#endif // ENABLE_TRACE_LOG
m_saveTimer.stop();
if (currentText().isEmpty() || !isEnabled()) {
if (cText.isEmpty() || !isEnabled()) {
return;
}
QString cText = currentText();
if (currentIndex() == -1) {
if (cIndex == -1) {
removeItem(-1);
}

// Check if the text is already listed
QSet<QString> querySet;
for (int index = 0; index < count(); index++) {
querySet.insert(itemText(index));
}
if (querySet.contains(cText)) {
// If query exists clear the box and use its index to set the currentIndex
int cIndex = findCurrentTextIndex();
int cIndex = findData(cText, Qt::DisplayRole);
setCurrentIndex(cIndex);
return;
} else {
Expand Down Expand Up @@ -532,16 +544,18 @@ void WSearchLineEdit::deleteSelectedComboboxItem() {
}

void WSearchLineEdit::deleteSelectedListItem() {
bool wasEmpty = currentIndex() == -1 ? true : false;
QComboBox::removeItem(view()->currentIndex().row());
// When the active query is removed the combobox would pick a sibling and
// trigger a search. Avoid that, just clear the search once.
// Also, there is a style update bug when changing the text of the hidden
// line edit, see updateClearAndDropdownButton()
int currViewindex = view()->currentIndex().row();
if (currViewindex == findCurrentTextIndex()) {
slotClearSearch();
}
QComboBox::removeItem(currViewindex);
// ToDo Resize the list to new item size
// Close the popup if all items were removed

// When an item is removed the combobox would pick a sibling and trigger a
// search. Avoid this if the box was empty when the popup was opened.
if (wasEmpty) {
QComboBox::setCurrentIndex(-1);
}
// Close the popup if all items were removed
if (count() == 0) {
hidePopup();
}
Expand Down Expand Up @@ -603,6 +617,16 @@ void WSearchLineEdit::updateClearAndDropdownButton(const QString& text) {
<< "updateClearAndDropdownButton"
<< text;
#endif // ENABLE_TRACE_LOG
// If the popup is open there's no need to further adjust the style, this is
// invoked by focusInEvent when the popup is closed.
// NOTE(ronso0) Also, when changing the text programmatically while the popup
// is open this style update would cause the QLineEdit to shrink in height
// (looks like a repaint without the padding from external stylesheets,
// noticed on Linux. Probably related)
if (view()->isVisible()) {
return;
}

// Hide clear button if the text is empty and while placeholder is shown,
// see disableSearch()
m_clearButton->setVisible(!text.isEmpty());
Expand All @@ -612,7 +636,7 @@ void WSearchLineEdit::updateClearAndDropdownButton(const QString& text) {
const int paddingPx = text.isEmpty() ? 0 : m_innerHeight;
const QString clearPos(layoutDirection() == Qt::RightToLeft ? "left" : "right");

// Hide the nonfunctional drop-down button if the search is disabled.
// Hide the nonfunctional drop-down button (set width to 0) if the search is disabled.
const int dropDownWidth = isEnabled() ? static_cast<int>(m_innerHeight * 0.7) : 0;

const QString styleSheet = QStringLiteral(
Expand Down