-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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: save queries across restarts, related tweaks & fixes #4458
Changes from all commits
6554e66
ac0c5da
d5f0398
40f33d6
d3f24fa
c3f328e
c5be3f3
a93b621
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -24,6 +24,8 @@ const QColor kDefaultBackgroundColor = QColor(0, 0, 0); | |
|
||
const QString kDisabledText = QStringLiteral("- - -"); | ||
|
||
const QString kSavedQueriesConfigGroup = QStringLiteral("[SearchQueries]"); | ||
|
||
constexpr int kClearButtonClearence = 1; | ||
|
||
inline QString clearButtonStyleSheet(int pxPadding, Qt::LayoutDirection direction) { | ||
|
@@ -74,9 +76,10 @@ void WSearchLineEdit::setDebouncingTimeoutMillis(int debouncingTimeoutMillis) { | |
s_debouncingTimeoutMillis = verifyDebouncingTimeoutMillis(debouncingTimeoutMillis); | ||
} | ||
|
||
WSearchLineEdit::WSearchLineEdit(QWidget* pParent) | ||
WSearchLineEdit::WSearchLineEdit(QWidget* pParent, UserSettingsPointer pConfig) | ||
: QComboBox(pParent), | ||
WBaseWidget(this), | ||
m_pConfig(pConfig), | ||
m_clearButton(make_parented<QToolButton>(this)) { | ||
qRegisterMetaType<FocusWidget>("FocusWidget"); | ||
setAcceptDrops(false); | ||
|
@@ -147,9 +150,15 @@ WSearchLineEdit::WSearchLineEdit(QWidget* pParent) | |
clearButtonSize.width() + m_frameWidth + kClearButtonClearence, | ||
layoutDirection())); | ||
|
||
loadQueriesFromConfig(); | ||
|
||
refreshState(); | ||
} | ||
|
||
WSearchLineEdit::~WSearchLineEdit() { | ||
saveQueriesInConfig(); | ||
} | ||
|
||
void WSearchLineEdit::setup(const QDomNode& node, const SkinContext& context) { | ||
auto backgroundColor = kDefaultBackgroundColor; | ||
QString bgColorName; | ||
|
@@ -219,17 +228,57 @@ void WSearchLineEdit::setup(const QDomNode& node, const SkinContext& context) { | |
tr("Use operators like bpm:115-128, artist:BooFar, -year:1990") + | ||
"\n" + tr("For more information see User Manual > Mixxx Library") + | ||
"\n\n" + | ||
tr("Shortcut") + ": \n" + tr("Ctrl+F") + " " + | ||
tr("Shortcuts") + ": \n" + | ||
tr("Ctrl+F") + " " + | ||
tr("Focus", "Give search bar input focus") + "\n" + | ||
tr("Ctrl+Backspace") + " " + | ||
tr("Clear input", "Clear the search bar input field") + "\n" + | ||
tr("Ctrl+Space") + " " + | ||
tr("Toggle search history", | ||
"Shows/hides the search history entries") + | ||
"\n" + | ||
tr("Delete or Backspace") + " " + tr("Delete query from history") + "\n" + | ||
tr("Esc") + " " + tr("Exit search", "Exit search bar and leave focus")); | ||
} | ||
|
||
void WSearchLineEdit::loadQueriesFromConfig() { | ||
if (!m_pConfig) { | ||
return; | ||
} | ||
const QList<ConfigKey> queryKeys = | ||
m_pConfig->getKeysWithGroup(kSavedQueriesConfigGroup); | ||
QSet<QString> queryStrings; | ||
for (const auto& queryKey : queryKeys) { | ||
const auto& queryString = m_pConfig->getValueString(queryKey); | ||
if (queryString.isEmpty() || queryStrings.contains(queryString)) { | ||
// Don't add duplicate and remove it from the config immediately | ||
m_pConfig->remove(queryKey); | ||
} else { | ||
// Restore query | ||
addItem(queryString); | ||
queryStrings.insert(queryString); | ||
} | ||
} | ||
} | ||
|
||
void WSearchLineEdit::saveQueriesInConfig() { | ||
if (!m_pConfig) { | ||
return; | ||
} | ||
// Delete saved queries in case the list was cleared | ||
const QList<ConfigKey> queryKeys = | ||
m_pConfig->getKeysWithGroup(kSavedQueriesConfigGroup); | ||
for (const auto& queryKey : queryKeys) { | ||
m_pConfig->remove(queryKey); | ||
} | ||
// Store queries | ||
for (int index = 0; index < count(); index++) { | ||
m_pConfig->setValue( | ||
ConfigKey(kSavedQueriesConfigGroup, QString::number(index)), | ||
itemText(index)); | ||
} | ||
} | ||
|
||
void WSearchLineEdit::updateStyleMetrics() { | ||
QStyleOptionComboBox styleArrow; | ||
styleArrow.initFrom(this); | ||
|
@@ -298,6 +347,13 @@ bool WSearchLineEdit::eventFilter(QObject* obj, QEvent* event) { | |
slotSaveSearch(); | ||
} | ||
} | ||
} else { | ||
if (keyEvent->key() == Qt::Key_Backspace || | ||
keyEvent->key() == Qt::Key_Delete) { | ||
// remove the highlighted item from the list | ||
deleteSelectedListItem(); | ||
return true; | ||
} | ||
} | ||
if (keyEvent->key() == Qt::Key_Enter) { | ||
if (findCurrentTextIndex() == -1) { | ||
|
@@ -335,6 +391,7 @@ void WSearchLineEdit::focusOutEvent(QFocusEvent* event) { | |
kLogger.trace() | ||
<< "focusOutEvent"; | ||
#endif // ENABLE_TRACE_LOG | ||
slotSaveSearch(); | ||
QComboBox::focusOutEvent(event); | ||
emit searchbarFocusChange(FocusWidget::None); | ||
if (m_debouncingTimer.isActive()) { | ||
|
@@ -408,23 +465,32 @@ void WSearchLineEdit::slotTriggerSearch() { | |
|
||
/// saves the current query as selection | ||
void WSearchLineEdit::slotSaveSearch() { | ||
int cIndex = findCurrentTextIndex(); | ||
#if ENABLE_TRACE_LOG | ||
kLogger.trace() | ||
<< "save search. Index: " | ||
<< cIndex; | ||
#endif // ENABLE_TRACE_LOG | ||
m_saveTimer.stop(); | ||
// entry already exists and is on top | ||
if (cIndex == 0) { | ||
if (currentText().isEmpty() || !isEnabled()) { | ||
return; | ||
} | ||
if (!currentText().isEmpty() && isEnabled()) { | ||
// we remove the existing item and add a new one at the top | ||
if (cIndex != -1) { | ||
removeItem(cIndex); | ||
} | ||
insertItem(0, currentText()); | ||
QString cText = currentText(); | ||
if (currentIndex() == -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(); | ||
setCurrentIndex(cIndex); | ||
return; | ||
} else { | ||
// Else add it at the top | ||
insertItem(0, cText); | ||
setCurrentIndex(0); | ||
while (count() > kMaxSearchEntries) { | ||
removeItem(kMaxSearchEntries); | ||
|
@@ -433,6 +499,9 @@ void WSearchLineEdit::slotSaveSearch() { | |
} | ||
|
||
void WSearchLineEdit::slotMoveSelectedHistory(int steps) { | ||
if (!isEnabled()) { | ||
return; | ||
} | ||
int nIndex = currentIndex() + steps; | ||
// at the top we manually wrap around to the last entry. | ||
// at the bottom wrap-around happens automatically due to invalid nIndex. | ||
|
@@ -443,6 +512,43 @@ void WSearchLineEdit::slotMoveSelectedHistory(int steps) { | |
m_saveTimer.stop(); | ||
} | ||
|
||
void WSearchLineEdit::slotDeleteCurrentItem() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The name is misleading, because it deletes the current item or the highlighted item. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I turned this into |
||
if (!isEnabled()) { | ||
return; | ||
} | ||
if (view()->isVisible()) { | ||
deleteSelectedListItem(); | ||
} else { | ||
deleteSelectedComboboxItem(); | ||
} | ||
} | ||
|
||
void WSearchLineEdit::deleteSelectedComboboxItem() { | ||
int cIndex = currentIndex(); | ||
if (cIndex == -1) { | ||
return; | ||
} else { | ||
slotClearSearch(); | ||
QComboBox::removeItem(cIndex); | ||
} | ||
} | ||
|
||
void WSearchLineEdit::deleteSelectedListItem() { | ||
bool wasEmpty = currentIndex() == -1 ? true : false; | ||
QComboBox::removeItem(view()->currentIndex().row()); | ||
// 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); | ||
} | ||
if (count() == 0) { | ||
hidePopup(); | ||
} | ||
} | ||
|
||
void WSearchLineEdit::refreshState() { | ||
#if ENABLE_TRACE_LOG | ||
kLogger.trace() | ||
|
@@ -464,6 +570,14 @@ void WSearchLineEdit::showPopup() { | |
setCurrentIndex(cIndex); | ||
} | ||
QComboBox::showPopup(); | ||
// When (empty) index -1 is selected in the combobox and the list view is opened | ||
// index 0 is auto-set but not highlighted. | ||
// Select first index manually (only in the list). | ||
if (cIndex == -1) { | ||
view()->selectionModel()->select( | ||
view()->currentIndex(), | ||
QItemSelectionModel::Select); | ||
} | ||
} | ||
|
||
void WSearchLineEdit::updateEditBox(const QString& text) { | ||
|
@@ -520,7 +634,9 @@ void WSearchLineEdit::slotClearSearch() { | |
kLogger.trace() | ||
<< "slotClearSearch"; | ||
#endif // ENABLE_TRACE_LOG | ||
DEBUG_ASSERT(isEnabled()); | ||
if (!isEnabled()) { | ||
return; | ||
} | ||
// select the last entry as current before cleaning the text field | ||
// so arrow keys will work as expected | ||
setCurrentIndex(-1); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will cause strange behaviour if there is more then 1 searchbar, but I don't know if this is even supported.
If there is a shared store for the queries there must be some sort of sync between the boxes and the config file, at least the object updated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
when a searchbar widget is created from the skin parser it's bound to the library widget.
So, if there is more than one searchbar only the last one is bound. didn't try that though. Edit both are bound and could work, but the queries are not sync'ed. Nothing to worry about since that is not a supported use case.There's another WSearchLineEdit in the dev tools, but it's created without passing the settings pointer, thus no queries are stored for that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Considering we will not be doing a major redesign of the QWidgets library to add a second WSearchLineEdit, I am willing to accept something kinda hacky.