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

[fix] Fix automatic trimming of search bar text #14497

Merged
merged 1 commit into from
Mar 28, 2025

Conversation

dhunstack
Copy link

search-no-trim.mov

As described in issue #14486, the search bar trims leading and trailing spaces in user's current text input after the auto-save timeout.
This PR fixes the issue by removing the problematic trimmed() call.

@github-actions github-actions bot added the ui label Mar 19, 2025
Copy link
Member

@ronso0 ronso0 left a comment

Choose a reason for hiding this comment

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

Thank you, the whitespace fix LGTM

But I think you need to adjust findCurrentTextIndex(), too. Currently the text (with spaces) is added to the list (without spaces) over and over again when you open the query list.

@@ -543,7 +543,7 @@ void WSearchLineEdit::slotTriggerSearch() {
/// saves the current query as selection
void WSearchLineEdit::slotSaveSearch() {
m_saveTimer.stop();
QString cText = currentText().trimmed();
QString cText = currentText(); // Keep original text with spaces for UI
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
QString cText = currentText(); // Keep original text with spaces for UI
// Keep original text for UI, potentially with trailing spaces
QString cText = currentText();

Copy link
Author

Choose a reason for hiding this comment

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

You're right, the query list was getting repeated updations.
I've fixed this, and the behaviour seems to be okay now.

@ronso0
Copy link
Member

ronso0 commented Mar 20, 2025

Btw the bug already exists in 2.5, so please rebase and force-push.
And I think you can squash the commits.

@ronso0 ronso0 added this to the 2.5.1 milestone Mar 20, 2025
@dhunstack
Copy link
Author

dhunstack commented Mar 20, 2025

Btw the bug already exists in 2.5, so please rebase and force-push. And I think you can squash the commits.

@ronso0
I squashed the commits and cherry-picked it onto 2.5 tag. Additional commit seems to be in this PR now, I'm not sure why that happened. Hadn't changed the base branch in PR, not an issue anymore.

@dhunstack dhunstack changed the base branch from main to 2.5 March 20, 2025 10:36
@ronso0
Copy link
Member

ronso0 commented Mar 21, 2025

Thank you, LGTM 👍
I'll do a final test tomorrow. Or some other @mixxxdj/developers does it.

FWIW I noticed a glitch in LateNight (PaleMoon) that may also exist in 2.5:
type a query, open query list -> searchbar shrinks in height (happens only on first call)
height is restored when the text cursor is moved in the searchbar.

Copy link
Member

@ronso0 ronso0 left a comment

Choose a reason for hiding this comment

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

LGTM, thank you!

@ronso0
Copy link
Member

ronso0 commented Mar 26, 2025

Oh, and we still need you sign the Mixxx Contributor Agreement and comment here when you have done so. It gives us permission to distribute your contribution under the GPL v2 or later license and the Apple Mac App Store. It is also helpful for us to have contact information for contributors in case we may need it in the future.

@ronso0
Copy link
Member

ronso0 commented Mar 26, 2025

Can you please rebase onto 2.5 to trigger CI?
Idk why it's not running, maybe because we didn't trigger all checks before the force-push? 🤷

As described in issue mixxxdj#14486, the search bar trims leading and
trailing spaces in user's current text input after the auto-save timeout.
This PR fixes the issue by removing the problematic `trimmed()` call.
Adds fix for keeping search history clean.
Adds fix for duplicated inserts in query box.

Signed-off-by: Anmol Mishra <[email protected]>
@dhunstack
Copy link
Author

LGTM, thank you!

Hi, thanks for approving the PR. I've been occupied with a conference deadline for this Friday, apologies for the delay.

Oh, and we still need you sign the Mixxx Contributor Agreement and comment here when you have done so. It gives us permission to distribute your contribution under the GPL v2 or later license and the Apple Mac App Store. It is also helpful for us to have contact information for contributors in case we may need it in the future.

Done

Can you please rebase onto 2.5 to trigger CI? Idk why it's not running, maybe because we didn't trigger all checks before the force-push? 🤷

I just force pushed after rebase, still doesn't trigger CI.
I see this message on my end actually, says some maintainer needs to approve the workflow?

ApprovalAwaiting

@ronso0
Copy link
Member

ronso0 commented Mar 27, 2025

I see this message on my end actually, says some maintainer needs to approve the workflow?

Yeah, now it's there again. weird..

@ronso0
Copy link
Member

ronso0 commented Mar 28, 2025

Thank you very much!

@ronso0 ronso0 linked an issue Mar 28, 2025 that may be closed by this pull request
@ronso0 ronso0 merged commit 780d118 into mixxxdj:2.5 Mar 28, 2025
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Search bar automatically trims spaces in the UI
2 participants