Skip to content

Session detail tweaks#1329

Merged
annavik merged 3 commits into
mainfrom
layout/session-detail-tweaks
May 29, 2026
Merged

Session detail tweaks#1329
annavik merged 3 commits into
mainfrom
layout/session-detail-tweaks

Conversation

@annavik
Copy link
Copy Markdown
Member

@annavik annavik commented May 29, 2026

Summary

Some follow up tweaks for #1327 .

List of Changes

  • Tweak content order for small screens
  • Exclude setting "snap to detections" after navigation improvements
  • Stabilize navigation layout for small screens

Summary by CodeRabbit

Release Notes

  • Improvements

    • Restructured session details page layout for better content organization
    • Process action now available only to logged-in users
    • Removed snap-to-detections setting from view preferences
  • Style

    • Updated settings icon button styling

Review Change Stack

@netlify
Copy link
Copy Markdown

netlify Bot commented May 29, 2026

Deploy Preview for antenna-ssec ready!

Name Link
🔨 Latest commit e885c9b
🔍 Latest deploy log https://app.netlify.com/projects/antenna-ssec/deploys/6a19ec4651b74300079c214f
😎 Deploy Preview https://deploy-preview-1329--antenna-ssec.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
🤖 Make changes Run an agent on this branch

To edit notification comments on pull requests, go to your Netlify project configuration.

@netlify
Copy link
Copy Markdown

netlify Bot commented May 29, 2026

Deploy Preview for antenna-preview ready!

Name Link
🔨 Latest commit e885c9b
🔍 Latest deploy log https://app.netlify.com/projects/antenna-preview/deploys/6a19ec46a5872200091b0edc
😎 Deploy Preview https://deploy-preview-1329--antenna-preview.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
Lighthouse
Lighthouse
1 paths audited
Performance: 58 (🔴 down 7 from production)
Accessibility: 81 (🔴 down 8 from production)
Best Practices: 92 (🔴 down 8 from production)
SEO: 92 (no change from production)
PWA: 80 (no change from production)
View the detailed breakdown and full score reports
🤖 Make changes Run an agent on this branch

To edit notification comments on pull requests, go to your Netlify project configuration.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 29, 2026

📝 Walkthrough

Walkthrough

This PR removes the snapToDetections setting across the session details page by updating ActivityPlot's capture selection logic, simplifying ViewSettings controls, making the Process component optional, and restructuring the session details layout into a two-column grid with repositioned header and content areas.

Changes

Snap-to-detections removal and session details redesign

Layer / File(s) Summary
Activity plot capture selection contract and logic
ui/src/pages/session-details/activity-plot/activity-plot.tsx
ActivityPlotProps now requires timeline and removes snapToDetections. Click handling is rewritten to use representativeCaptureId directly when present; otherwise it falls back to findClosestCaptureId with snapToDetections: false, removing prior conditional behavior.
ViewSettings control and contract simplification
ui/src/pages/session-details/view-settings.tsx
Component removes the session prop dependency and snapToDetections setting from the props contract. The snap-to-detections checkbox control and associated enable/disable logic are removed from the popover UI. Settings icon button variant changes from ghost to outline.
Session details page integration and layout restructure
ui/src/pages/session-details/process/process.tsx, ui/src/pages/session-details/session-details.tsx
Session details page drops snapToDetections from settings state initialization. Process component becomes optional; when capture is missing, it renders a disabled button fallback. PageHeader layout is refactored: Process is moved into the header and conditionally rendered when logged in. Main content area is restructured from flex to two-column grid with repositioned tabs, capture panel, and controls. ActivityPlot callsite no longer passes snapToDetections.

Sequence Diagrams

No sequence diagrams generated. The changes primarily involve removing a setting flag, simplifying component contracts, and restructuring layout — patterns that do not meet the criteria for visualization (interactions are reduced rather than introduced, and the control flow is straightforward).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • RolnickLab/antenna#1327: Part of the same session-details redesign affecting activity-plot, process, view-settings, and session-details components, particularly around snapToDetections and capture selection behavior.

Suggested reviewers

  • mihow

Poem

🐰 A hop through the UI, a snap setting fades,
No more detections to toggle in shades,
ActivityPlot learns its timeline with grace,
ViewSettings grows lean in its cleaner place,
Session details stands tall in its gridded two-column space! ✨

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description covers the main points but lacks detailed explanations. It's missing testing instructions, deployment notes, and the checklist is incomplete, deviating from the template requirements. Complete the description by adding 'How to Test the Changes' section with specific test scenarios, 'Deployment Notes' if applicable, and fill out the checklist items to indicate verification of changes.
Title check ❓ Inconclusive The title 'Session detail tweaks' is vague and generic, using non-descriptive language that doesn't convey meaningful information about the specific changes made to the codebase. Consider a more specific title that highlights the main changes, such as 'Remove snapToDetections setting and restructure session details layout for mobile' or 'Refactor session details layout and remove snap-to-detections feature'.
✅ Passed checks (3 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch layout/session-detail-tweaks

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@ui/src/pages/session-details/process/process.tsx`:
- Around line 21-35: The Process component initializes pipelineId from async
project data but never updates it when project arrives; add a useEffect in
Process that watches project (or
project?.settings.defaultProcessingPipeline?.id) and calls
setPipelineId(project.settings.defaultProcessingPipeline.id) when a default
exists and pipelineId is not already set/overridden; reference the Process
component, the pipelineId state and setPipelineId, and useProjectDetails to
locate where to add the effect so the default pipeline is synced after async
load.

In `@ui/src/pages/session-details/session-details.tsx`:
- Around line 128-161: The mobile reading/focus order is wrong because the
sidebar Box uses CSS order-last; remove reliance on visual ordering and change
the JSX so the Box element appears in the DOM in the intended mobile reading
order (i.e., move the <Box className="order-last ..."> node to after the main
content in the JSX), then keep only the desktop reordering class (e.g., use
md:order-first on the Box) so desktop layout stays the same; update any
className references (remove order-last, keep md:order-first) and verify
components inside (Tabs.Root, Tabs.List, Tabs.Trigger, Tabs.Content,
SessionInfo, CaptureInfo, SessionPlots) remain unchanged.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: cbfc3f63-b932-4eb5-a367-9173f1ba3af4

📥 Commits

Reviewing files that changed from the base of the PR and between 257f270 and e885c9b.

📒 Files selected for processing (4)
  • ui/src/pages/session-details/activity-plot/activity-plot.tsx
  • ui/src/pages/session-details/process/process.tsx
  • ui/src/pages/session-details/session-details.tsx
  • ui/src/pages/session-details/view-settings.tsx

Comment thread ui/src/pages/session-details/process/process.tsx
Comment thread ui/src/pages/session-details/session-details.tsx
@annavik annavik merged commit 4aafddf into main May 29, 2026
7 checks passed
@annavik annavik deleted the layout/session-detail-tweaks branch May 29, 2026 19:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant