Improve bookmark controls #31806
Merged
Improve bookmark controls #31806
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
- Bookmark menu items get disabled when they would do nothing. - Bookmark deletion only deletes the closest bookmark instead of all of them within the proximity of 2 seconds to current clock time. Action is only however *enabled* within 2 seconds of a bookmark. Additionally, logic was moved out of `Editor` because it's a huge class and I dislike huge classes if they can be at all avoided.
Found in testing of previous commit. This would break seeking between bookmarks. Reproduction steps on `master`: - open map with bookmark - delete the first bookmark - undo the deletion of the first bookmark - seek to previous bookmark will now always seek to the first bookmark rather than closest preceding regardless of current clock time
Glad you did this, as I was going to mention it in the last PR. |
|
This seems fine. Next up for bookmarks is going to be display in the timeline (probably as diamonds to give them a distinguishing visual separation) and some kind of feedback when adding / removing I guess. |
peppy
approved these changes
Feb 6, 2025
|
Disclosure: pushed d9b370e to master because I forgot to push here before merging. |
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.
Reproduction steps on
masterfor the aforementioned bug:Additionally, bookmark logic was moved out of
Editorbecause it's a huge class and I dislike huge classes if they can be at all avoided and in this instance they can. Not sure if you'll like that, can move back if that is preferred.