-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Convert all tool Popups to floating windows #1658
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
…opups Additional changes: - Added a 'Pin' button to the floating popup's toolbar. When a tool is pinned, the popup will not close automatically after the action is executed. - Added a virtual function to the `ViewHexEditor::Popup` class, allowing implementations to specify a tool window title.
Before this change, the modifications to the tool popup UIs did not respect the user's scaling factor settings.
…dd missing EndPopup
…de to ViewHexEditor::drawPopup()
…window when the user isn't hovering over it In the previous approach, I was using ImGuiWindowFlags_NoBackground combined with an explicitly drawn background rectangle to correctly set the window transparency within ImGuiExt::Begin/EndHoveringPopup(). However with this approach, there was a small but noticeable gap between the window's external borders and the background rectangle, due to the configured default padding. With this new approach, the decision to apply the transparency will always be delayed by one frame, but theis way the whole window can be set to being transparent before ImGuiExt::BeginHoveringPopup().
…e user isn't hovering over
…ect until the user hovers over it
…d PopupWindowAlpha value
SparkyTD
commented
May 8, 2024
SparkyTD
commented
May 8, 2024
This fix restores the previously decided behavior ensuring that newly opened popup windows are initially non-transparent, until the user hovers over them for the first time.
I found a bug that makes popup windows unusable after they've been dragged outside of the root window. Repro:
EDIT: The commit below (72701a6) fixes this issue, but more rigorous testing might be useful. |
…isappear on click This if branch handles the case when the user clicks outside of the currently visible popup, and it loses focus. But if the popup loses focus while `ImGui::IsWindowHovered()` still returns true, it means that the focus loss was not caused by the user clicking away, so it should not be closed yet. Since the focus loss was only registered when clicking on the background / title bar of the popup, and not any of its child widgets, this could be an ImGui bug.
WerWolv
reviewed
May 9, 2024
…ly multiplying with the scale factor
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
In previous versions of ImHex, all tool windows were implemented as static popups fixed in the upper left position of the hex view. This PR refactors all tool popups to use floating windows that can be dragged around by the user, or closed with a dedicated close button on the title bar. These popup also support a stylable transparency when the user is not hovering their mouse over the window.
Implementation description
I rewrote the logic in
ViewHexEditor::drawPopup()
to use a customImGuiExt::BeginHoveringPopup
function for rendering the popup windows. This new function is an almost exact replica of the built-inImGui::BeginPopupModal
, except it does also displays the default window title bar with a close button.A second custom function,
ImGuiExt::PopupTitleBarButton
was also added for rendering small icon-based buttons into the title bar of the parent popup window. This new function was used to implement an optional "Pinning" feature that individual popup implementations can specify. If a window is pinned, it won't close automatically when its main action is executed. For example, the "Select" button on the Select dialog will close the popup by default, unless the window is pinned.Screenshots
Popup dialogs before:
Popup dialogs after:
Recording.2024-05-08.233435.mp4
Additional things
styles.imhex.popup-alpha
style, making the transparency effect configurable, including the ability to disable the effect completely by settingpopup-alpha
to1.0
.Possible changes and improvements