web: add multi-selection and inspector cycling with pulse animation#10601
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces multi-selection capabilities to the web inspector, allowing users to Shift+click to add items to a selection set and cycle through them using a new navigation bar. It also adds a visual pulsing animation to highlight the selected object's bounding box. The backend changes support tracking the selection set and handling next/prev requests, accompanied by comprehensive unit tests. Feedback on the pull request highlights a logical bug where Shift+clicking on an empty area clears the selection instead of preserving it, along with a corresponding test update. Additionally, several defensive programming improvements are suggested in the JavaScript code, such as validating the selection index type, checking if app.map is still valid before removing layers or closing popups, and restoring the inspector's state if a selection cycle request fails.
Shift+click adds objects to a SelectionSet (mirroring the Qt GUI),
with all selected objects highlighted simultaneously on the layout
tiles. The inspector shows Previous/Next buttons and a position
label ("2 / 5") when multiple objects are selected, cycling through
the set. Each selection change triggers a brief pulse animation on
the object's bounding box so the user can see which object the
inspector is now showing.
Backend: SessionState gains a gui::SelectionSet and iterator.
handleSelect reads an optional add_to_selection flag from the
request JSON. New select_next / select_prev request types advance
the iterator with wrap-around. All inspect/inspect_back responses
now include selection_count and selection_index metadata.
Frontend: main.js detects shiftKey and sets add_to_selection.
inspector.js adds cycleSelection(), the nav bar UI, and
pulseHighlight(). style.css adds the selection-pulse keyframe
animation and nav bar layout.
Signed-off-by: Matt Liberty <mliberty@precisioninno.com>
ceda78d to
4de04f4
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4de04f43fb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
gadfort
left a comment
There was a problem hiding this comment.
Works, but certainly exposes the latency in drawing, switching selections seems to retrigger a whole host of drawing.
|
Ok but that is separate issue. |
Address codex feedback on the multi-selection feature: - handleSelectionCycle: clear hover_rects/timing_rects/timing_lines so stale hover/timing overlays don't bleed through after Next/Previous. - handleInspect: realign selection_itr with the linked target so selection_index matches the rendered payload and the next cycle starts from the correct object (or end() to suppress nav when the link leaves the selection set). - main.js: skip the empty-hit inspector/highlight clear when add_to_selection is set, since the server preserves the multi-selection on shift-click misses. Also add <iterator> include for std::distance (clang-tidy misc-include-cleaner). Signed-off-by: Matt Liberty <mliberty@precisioninno.com>
65d99b9 to
e5d8cc3
Compare
Summary
SelectionSet(mirroring the Qt GUI), with all selected objects highlighted simultaneously on the layout tilesDetails
Backend:
SessionStategains agui::SelectionSetand iterator.handleSelectreads an optionaladd_to_selectionflag. Newselect_next/select_prevrequest types advance the iterator with wrap-around. All inspect responses includeselection_countandselection_indexmetadata.Frontend:
main.jsdetectsshiftKeyand setsadd_to_selection.inspector.jsaddscycleSelection(), the nav bar UI, andpulseHighlight().style.cssadds theselection-pulsekeyframe animation.Test plan
TestRequestHandler.cppcovering selection set population, next/prev cycling, wrap-around, empty set, navigation history clearing, and metadata in inspect/inspect_back responsestest-inspector.jscovering nav bar visibility, label formatting, and button request wiringctest -R web,bun test)