-
Notifications
You must be signed in to change notification settings - Fork 244
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
Introduce a profile selector dropdown #282
Merged
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
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
This adds much better UI for selecting different profiles within a single import. ![Kapture 2020-05-30 at 21 34 06](https://user-images.githubusercontent.com/150329/83344564-595ce400-a2bd-11ea-8306-e5d8f647b65e.gif) You can now hover over the middle of the toolbar or hit `t` on your keyboard to bring up the profile selector. From there, you can use fuzzy-find to switch to the profile you want, and hit "enter" to select it. The up and down arrow keys can be used while the profile selector filter input is focused to move through the list of profiles. I think the "next" and "prev" buttons are now totally useless, so I removed them. Fixes jlfwong#167
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 adds much better UI for selecting different profiles within a single import.
You can now hover over the middle of the toolbar or hit
t
on your keyboard to bring up the profile selector. From there, you can use fuzzy-find to switch to the profile you want, and hit "enter" to select it. The up and down arrow keys can be used while the profile selector filter input is focused to move through the list of profiles.I think the "next" and "prev" buttons are now totally useless, so I removed them.
Fixes #167