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.

Sign up for GitHub

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

Jump to bottom

Add countdown to showing command palette no matches message #3399

Merged
merged 3 commits into from
Sep 28, 2023
Merged

Add countdown to showing command palette no matches message #3399

merged 3 commits into from
Sep 28, 2023

Conversation

guy-av
Copy link
Contributor

@guy-av guy-av commented Sep 25, 2023 •
edited
Loading

The countdown protects from spamming the message and helping the palette feel smoother when there are no matches.

  • Docstrings on all new or modified functions / classes
  • Change test_no_results to wait for the countdown

Resolves #3278

@guy-av
Copy link
Contributor Author

guy-av commented Sep 25, 2023

This PR tries to follow suggested changes noted in this comment.

@guy-av guy-av force-pushed the fix/palette-no-results-flicker branch from 4fdc596 to 3e0b329 Compare September 25, 2023 16:55
davep
davep reviewed Sep 27, 2023
View reviewed changes
Copy link
Contributor

@davep davep left a comment

Choose a reason for hiding this comment

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

Looking good; thanks. Couple of very small tweaks and one question.

Also, very small general observation (which really isn't a problem, very much appreciate the contribution, but worth a mention in regards to FOSS contributions in general): this issue was already assigned so it would be nice to check with the assignee first if it's okay to jump in.

src/textual/command.py Show resolved Hide resolved
src/textual/command.py Outdated Show resolved Hide resolved
src/textual/command.py Outdated Show resolved Hide resolved
@davep davep added enhancement New feature or request Task labels Sep 27, 2023
@guy-av
Copy link
Contributor Author

guy-av commented Sep 27, 2023

Looking good; thanks. Couple of very small tweaks and one question.

Also, very small general observation (which really isn't a problem, very much appreciate the contribution, but worth a mention in regards to FOSS contributions in general): this issue was already assigned so it would be nice to check with the assignee first if it's okay to jump in.

Sure thing!

@guy-av
Add countdown to showing command palette no matches message
ee5b37f 
The countdown protects from spamming the message and helping the palette feel smoother when there are no matches
@guy-av
test_no_results() wait for message to show
0289a20 
The "no matches found" message in the command palette show after a specific delay, so make the test wait the same delay
@guy-av guy-av force-pushed the fix/palette-no-results-flicker branch from 3e0b329 to 0289a20 Compare September 27, 2023 19:52
@guy-av guy-av requested a review from davep September 27, 2023 20:17
davep
davep approved these changes Sep 28, 2023
View reviewed changes
Copy link
Contributor

@davep davep left a comment

Choose a reason for hiding this comment

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

Looks good to me! Thanks again.

I've also added an entry to the CHANGELOG.

@davep davep merged commit dee7459 into Textualize:main Sep 28, 2023
22 checks passed
@guy-av guy-av deleted the fix/palette-no-results-flicker branch September 28, 2023 08:38
@davep davep mentioned this pull request Nov 20, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@davep davep davep approved these changes

Assignees
No one assigned
Labels
enhancement New feature or request Task
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Command palette flicker when there are no results
2 participants
@guy-av @davep