Fix jump scroll offset #1660
Merged
Fix jump scroll offset #1660
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
…n necessary When the cursor jumps to a new byte, the scroll offset should only change when the selected byte falls outside the scrolling viewport.
…incorrect offset is entered
WerWolv
reviewed
May 9, 2024
WerWolv
pushed a commit
that referenced
this pull request
May 10, 2024
### Problem description Merging my previous PRs, #1660 and #1658 has resulted in a conflict causing an application crash due to a missing `ImGui::BeginDisabled();` in the `PopupSelect` class. Specifically, none of the code related to offset validation made it into the final merge. This PR fixes the crash by reintroducing the deleted lines. ### Additional things The nightly release build seems to be unaffected by the crash, most likely because ImGui's `DisabledStack` assertions are only enforced in Debug mode. The offset validation was still missing before this fix.
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.
Problem description
This PR offers two improvements:
When selecting / jumping to an offset that falls within the current viewport of the scroll view, the scroll offset will no longer force the selected byte to the top of the view. Instead, the scroll offset will only be changed if the selected byte is outside the current view.
In case a wrong offset is entered into the Select or Goto dialog (e.g. and offset beyond EoF), the dialog's button will be disabled.
Implementation description
For the first change, I modified the logic that recalculates the
m_scrollPositionbased on the current byte offset.For the second change, I added validation logic to both popups to ensure that the entered offsets are valid (using
provider->getActualSize()). In case of the Select popup, I wrapped the button into anImGui::Begin/EndDisabledto enforce the validation check.