Refactor: UI restructure, notifications, save dialog, and categorical LUT fix#32
Merged
Conversation
…UI helpers - Move Edit Mode (field selection, expression, assign) from left pipeline drawer into a new _build_edit_assign_panel() in the right Edit tab - Extract _build_selection_tools_panel() from inline code in the Edit tab - Move Geometry mode selector from Field Assignment into Edit Tools panel, above Selection mode — it controls picking, not assignment - Reorder Edit Tools: Pick/Rotate, Select All/Clear, selection count, Geometry mode, Selection mode, Selection behavior, Grow/angle, status - Add docs/ui_structure.md with full component tree and planned module split - Link ui_structure.md from README Architecture section - Update e2e tests: wait_for_selector anchored to "Edit Tools" label
…kbar Replaces 9 scattered alert state pairs (error_message, upload_status, edit_status, save_status, state_status, edit_apply_status, edit_selection_status, color_controls_status, pv_runtime_message display) with a single VSnackbar driven by notification_message/type/show. notify() helper in notifications.py is the sole write path; snackbar auto-dismisses after 5 s. Success toasts removed from self-evident toggle operations (color bar, orientation axes, categorical) to avoid covering the viewport during E2E screenshot assertions.
Save dialog refactor: - Remove inline Output filename field from toolbar - Replace direct save button actions with a two-step flow: button opens a named save dialog (save_dialog_action = "save"|"commit"), pv_confirm_save_dialog routes to save_paraview_output or _commit_edit_session with the filename passed explicitly - Add pv_confirm_save_dialog / pv_cancel_save_dialog controllers - Add save_dialog / save_dialog_action to state_setup - Pass filename kwarg through file_operations.save_paraview_output and resolve_paraview_output_path instead of reading state as side effect; overwrite retry uses save_overwrite_target (the conflicting path) not save_filename - Update unit and e2e tests for the dialog-based flow Toolbar buttons: - Buttons use small VBtn with icon + small text label - docs/ui_structure.md: add Toolbar button flows diagram Categorical LUT fix: - Re-apply categorical LUT annotations whenever the active LUT is already marked categorical after an array/range/preset change, so the color bar keeps reflecting current data values without requiring a UI toggle - Remove legacy hasattr guards in _disable_scalar_coloring; access display.LookupTable / ColorArrayName directly - Add square_mat_12.vtk and square_mat_345.vtk test meshes with MaterialID fields for categorical coloring coverage
pcolt
marked this pull request as draft
May 20, 2026 11:32
Collaborator
Author
|
@luca-heltai this is marked as draft because I may add some other refactors or fixes but I could also split in different PRs. What is ready is PR #31 which needs to merge or get reviewed first |
The test was intermittently failing on CI after the new categorical test added resource pressure, causing the 1s sleep to be insufficient for the ParaView render to complete. _wait_for_foreground polls every 250ms up to 8s, so the assertion fires as soon as content appears rather than at a fixed deadline.
Two timing-sensitive failures on CI: - categorical_toggle: assert _switch_checked immediately after wait_for_selector fired before the Trame state update arrived in the browser; replaced with _wait_for_switch_checked (polls up to 8s). - show_faces: _wait_for_foreground returned the previous full-mesh render immediately because the viewport already had content; added narrower_than so the poll waits until the bbox shrinks to a fraction of the full width before accepting the faces-only render.
Extend the categorical LUT staleness fix to cover active-node changes: set_active_node now calls _refresh_categorical_annotations so the color bar reflects the new source's values when the user clicks between pipeline nodes, without requiring a manual toggle. Extract the repeated 3-line conditional into _refresh_categorical_annotations so both apply_coloring and set_active_node share a single call site. Extend test_paraview_categorical_annotations_updated_on_file_switch with pipeline-switch steps: after the file-load regression check, click file A then file B in the pipeline browser and verify the render is stable before and after a categorical toggle. Add _wait_for_stable_viewport helper that polls until two consecutive screenshots show no significant change, avoiding the race between the Trame state push and the ParaView render push.
pcolt
marked this pull request as ready for review
May 22, 2026 08:59
Contributor
|
I like this. I think we have a bug in two dimensions. For a refined two dimensional grid, surface selection does not seem to work. But it's the same in the main branch. |
Collaborator
Author
|
Could you confirm the bug or make a video perhaps? Because I am not able to reproduce it |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Overview
Edit panel restructure — field assignment controls moved from the left pipeline drawer into the right inspector Edit tab; UI helpers extracted; Edit Tools controls reordered;
docs/ui_structure.mdadded with full component tree.Centralize notifications — 9 scattered alert state pairs replaced with a single
VSnackbardriven bynotify()innotifications.py; auto-dismisses after 5 s.Save dialog, toolbar polish, categorical LUT fix — inline filename field replaced with an explicit save dialog (pre-filled, routes
save_dialog_action = "save" | "commit"); toolbar buttons use icon + small text; categorical LUT annotations re-applied on array/range/preset changes so the color bar keeps reflecting data values without toggling.Summary
_build_edit_assign_panel()and_build_selection_tools_panel()into the right Edit tab; reorder Edit Tools controlsdocs/ui_structure.mdwith component tree and planned module splitnotification_message/notification_type/notification_show+VSnackbar; addnotifications.pywithnotify()helper_apply_pending_save_filename/_save_active_datahelpers;pv_save_active_dataopens a dialog (save_dialog_action = "save")pv_confirm_save_dialog(filename)andpv_cancel_save_dialog();pv_commit_edit_sessionopens same dialog withsave_dialog_action = "commit"save_paraview_output/resolve_paraview_output_pathtake explicitfilenamekwarg; overwrite retry usessave_overwrite_targetsave_dialog/save_dialog_actiontostate_setup.py_configure_categorical_lookup_tableafter array/range/preset changes when LUT is already categorical; removehasattrguards in_disable_scalar_coloringtest_data/square_mat_12.vtkandsquare_mat_345.vtkfor categorical coloring coverageTest plan
conda run -n coral-paraview pytest -q tests/ --ignore-glob=tests/test_e2e*.py(165 pass)conda run -n coral-paraview pytest -q tests/test_e2e_edit_selection_playwright.py