-
Notifications
You must be signed in to change notification settings - Fork 255
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
Implement search highlighting in time order & left heavy views #297
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…eedscope search input focused
… it with a search query set then clearing that search query
jlfwong
changed the title
Implement search highlighting in the flamechart views
Implement search highlighting in time order & left heavy views
Jul 20, 2020
Closed
jlfwong
added a commit
that referenced
this pull request
Oct 21, 2022
…hing frames (#407) In #297, I re-used the fuzzy matching logic I implemented in #282 for profile selection. Based on feedback from several people in #352, this is surprising behavior. Upon reflection, this should have been obvious -- I hijacked the Ctrl/Cmd+F browser behaviour, so I should try to replicate the expected behaviour there as closely as possible. Given more patience, I also would've done some user research :) This PR updates this logic to try to more closely match browser behaviour. This means case-insensitive, exact-substring matching. I've left the fuzzy matching alone for profile selection since that doesn't attempt to mimic browser behaviour. The non-fuzzy matching feels slightly odd to me given the filtering behaviour on the sandwich view, but I think consistency across this find UI is important. Here are the before & after results when searching for the string "ca" in the example profile. |Before|After| |-|-| |<img width="1791" alt="image" src="https://user-images.githubusercontent.com/150329/197232741-6d1d7a8a-8b8c-4a4f-98e3-2c043fd7efd5.png">|<img width="1789" alt="image" src="https://user-images.githubusercontent.com/150329/197232694-82697b68-ca15-49e7-887b-2606646ee5e9.png">| Fixes #352 Supersedes #403
jackerghan
pushed a commit
to jackerghan/speedscope
that referenced
this pull request
Jul 28, 2023
…ng#297) This implements the next step towards full featured search in speedscope: visual highlighting of matching search results in the time ordered & left heavy views. This doesn't yet add the ability to click prev/next to select the next matching element in the editor, but I'm still planning on doing something like that. I haven't figured out yet what I want the user experience to be like for that. ![speedscope-flamegraph-search](https://user-images.githubusercontent.com/150329/87898991-9ebba900-ca04-11ea-9bd9-31ad8d4c6d2a.gif) This works towards fixing jlfwong#38
jackerghan
pushed a commit
to jackerghan/speedscope
that referenced
this pull request
Jul 28, 2023
…hing frames (jlfwong#407) In jlfwong#297, I re-used the fuzzy matching logic I implemented in jlfwong#282 for profile selection. Based on feedback from several people in jlfwong#352, this is surprising behavior. Upon reflection, this should have been obvious -- I hijacked the Ctrl/Cmd+F browser behaviour, so I should try to replicate the expected behaviour there as closely as possible. Given more patience, I also would've done some user research :) This PR updates this logic to try to more closely match browser behaviour. This means case-insensitive, exact-substring matching. I've left the fuzzy matching alone for profile selection since that doesn't attempt to mimic browser behaviour. The non-fuzzy matching feels slightly odd to me given the filtering behaviour on the sandwich view, but I think consistency across this find UI is important. Here are the before & after results when searching for the string "ca" in the example profile. |Before|After| |-|-| |<img width="1791" alt="image" src="https://user-images.githubusercontent.com/150329/197232741-6d1d7a8a-8b8c-4a4f-98e3-2c043fd7efd5.png">|<img width="1789" alt="image" src="https://user-images.githubusercontent.com/150329/197232694-82697b68-ca15-49e7-887b-2606646ee5e9.png">| Fixes jlfwong#352 Supersedes jlfwong#403
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This implements the next step towards full featured search in speedscope: visual highlighting of matching search results in the time ordered & left heavy views. This doesn't yet add the ability to click prev/next to select the next matching element in the editor, but I'm still planning on doing something like that. I haven't figured out yet what I want the user experience to be like for that.
This works towards fixing #38