Skip to content

fix(web): resolve selection issues in SelectionManager and History view - #859

Merged
kyasbal merged 3 commits into
GoogleCloudPlatform:mainfrom
kyasbal:push-wzzvpkwspwyu
Aug 4, 2026
Merged

fix(web): resolve selection issues in SelectionManager and History view#859
kyasbal merged 3 commits into
GoogleCloudPlatform:mainfrom
kyasbal:push-wzzvpkwspwyu

Conversation

@kyasbal

@kyasbal kyasbal commented Aug 3, 2026

Copy link
Copy Markdown
Member

Related Issue

Fixes #858

Summary

This PR addresses several non-intuitive selection behaviors in SelectionManager and the History (Diff) view when interacting with logs and revisions across timeline hierarchies.
Previously, SelectionManager relied on bidirectional synchronization loops and multiple ignoreXxxSelect control flags, which caused log selections to be inadvertently cleared—especially when a log belonged to multiple timelines or a descendant timeline. Furthermore, the History view did not update its target timeline when inspecting a revision belonging to a child resource.

Key Changes

  • SelectionManager Refactoring & Simplification
    • Removed circular synchronization methods (synchronizeSelection, changeSelectionByXxxInternal) and over 100 lines of complex control flags (ignoreLogSelect, ignoreResourceSelect, ignoreTimelineSelect).
    • Rebuilt selection methods (onSelectTimeline, onSelectLog, onSelectRevision, onSelectEvent) into clear, unidirectional state updates.
  • Support for Selecting Logs in Child Timelines
    • Expanded the lookup and validation scope from the single selectedTimeline to selectedTimelinesWithChildren().
    • Users can now seamlessly click and select any log belonging to a descendant timeline while a parent timeline is selected, without the selection being rejected or cleared.
  • History View Context-Aware Rendering
    • Updated previousOfSelectedRevision to compute from revision.timeline rather than selectedTimeline(), correctly resolving the previous revision even for child timeline elements.
    • Updated DiffSmartComponent (selectedTimeline) to prioritize the timeline of the currently selected revision or log. Consequently, the History view now accurately displays the revision history and diffs of selected child resources.
  • Unit Tests
    • Added unit test coverage in selection-manager.service.spec.ts verifying log selection within child timelines and proper selection cleanup behavior.

Verification

  • Automated Tests:
    • Ran make test-web: All 724 unit tests passed successfully.
    • Ran make format-web, make lint-web, and make build-storybook: All checks passed with 0 issues.
  • Pre-commit Check:
    • Ran make pre-commit: Verified formatting and static analysis across all modified files (0 issues).

- Refactor SelectionManager to remove circular synchronization and control flags (ignoreXxxSelect).
- Support selecting logs and revisions in descendant timelines when a parent timeline is selected by expanding lookup and validation scope to selectedTimelinesWithChildren().
- Compute previousOfSelectedRevision using revision.timeline instead of selectedTimeline() to correctly resolve previous revisions for child timelines.
- Update DiffSmartComponent (selectedTimeline) to prioritize the timeline of the currently selected revision or log so that the History view correctly renders descendant revision histories.
- Update unit tests in selection-manager.service.spec.ts to verify child timeline selection and cleanup behaviors.
@kyasbal kyasbal added the bug Something isn't working label Aug 3, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request refactors the selection management logic in SelectionManager to improve how timelines, logs, and revisions are selected and validated, including updating the selectedTimeline computed signal in DiffSmartComponent and adding corresponding unit tests. The review feedback suggests a usability improvement in onSelectLog to fall back to searching all filtered timelines when an unrelated log is selected while a timeline is already active, rather than clearing the selection entirely, along with an additional unit test to verify this behavior.

Comment thread web/src/app/services/selection-manager.service.ts
Comment thread web/src/app/services/selection-manager.service.spec.ts
@kyasbal
kyasbal marked this pull request as ready for review August 4, 2026 08:14
@kyasbal
kyasbal merged commit 8a67324 into GoogleCloudPlatform:main Aug 4, 2026
16 checks passed
@kyasbal
kyasbal deleted the push-wzzvpkwspwyu branch August 4, 2026 08:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Inconsistent selection behavior when selecting logs or revisions within selected timelines and child hierarchies

1 participant