Skip to content

Conversation

@Bhoomika2905
Copy link
Collaborator

@Bhoomika2905 Bhoomika2905 commented Nov 30, 2025

Pull Request

Description

Adds violin plot support in maidr, enabling accessible navigation and exploration of violin plot visualizations. Violin plots combine kernel density estimation (KDE) curves with box plot statistics. This implementation provides specialized navigation for both layers and maintains architectural consistency by removing plot-specific metadata from the general data structure.

Changes Made

This PR implements comprehensive support for violin plots in maidr, enabling accessible navigation and exploration of violin plot visualizations. Violin plots combine kernel density estimation (KDE) curves with box plot statistics, and this implementation provides specialized navigation behavior for both layers.

Core Features
Violin Plot Navigation: Specialized navigation for violin plots with swapped directional controls:
KDE Layer: Left/Right arrows switch between violins (row changes), Up/Down arrows traverse along the density curve (col changes)
Box Layer: Standard navigation with support for preserving Y values when switching between layers
Layer Switching: Seamless switching between KDE and box layers while preserving X and Y values for context continuity

Screenshots (if applicable)

Recording 2025-11-30 at 02 43 30

Checklist

  • I have read the Contributor Guidelines.
  • I have performed a self-review of my own code and ensured it follows the project's coding standards.
  • I have tested the changes locally following ManualTestingProcess.md, and all tests related to this pull request pass.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have updated the documentation, if applicable.
  • I have added appropriate unit tests, if applicable.

@Bhoomika2905 Bhoomika2905 requested a review from nk1408 November 30, 2025 01:01
@github-actions
Copy link

⚠️ Changes detected in model-related files

This PR contains changes to files that affect the model layer and unit tests. Before merging, please ensure:

  1. Run the unit tests locally:

    npm run test
  2. Update tests if necessary

@github-actions
Copy link

⚠️ Changes detected in model-related files

This PR contains changes to files that affect the model layer and unit tests. Before merging, please ensure:

  1. Run the unit tests locally:

    npm run test
  2. Update tests if necessary

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR implements comprehensive support for violin plots in maidr by introducing a new ViolinKdeTrace class and enhancing the BoxTrace class to handle violin box plots. Violin plots combine kernel density estimation (KDE) curves with box plot statistics, requiring specialized navigation that allows users to explore both the density distribution and statistical summaries.

Key changes include:

  • New ViolinKdeTrace class that extends SmoothTrace with swapped navigation (left/right switches between violins, up/down traverses the density curve)
  • Enhanced BoxTrace with violin-specific behavior for layer switching with Y-value preservation
  • Dynamic volume control in audio service using volumeScale based on density values
  • Context layer switching logic that preserves both X and Y positions when moving between violin layers

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 14 comments.

Show a summary per file
File Description
src/model/violinKde.ts Implements new ViolinKdeTrace class with specialized navigation, audio (density-based pitch/volume), and layer switching with Y-value preservation
src/model/box.ts Adds violin box plot detection and behavior, including Y-value preservation during layer switches and bottom-point reset when changing violins
src/model/smoothtraceFactory.ts Adds violin plot detection logic to create ViolinKdeTrace when BOX + SMOOTH layers are present
src/model/factory.ts Passes allLayers parameter to enable violin plot detection in trace factories
src/model/context.ts Implements onSwitchFrom pattern for custom layer switching behavior with Y-value preservation
src/model/plot.ts Adds optional onSwitchFrom method to Trace interface for custom layer switching
src/service/audio.ts Adds volumeScale and volumeMultiplier parameters for dynamic volume control based on data characteristics
src/service/text.ts Excludes cross value announcement for violin box plots during layer switches
src/type/state.ts Adds volumeMultiplier and volumeScale properties to AudioState for dynamic volume control

@github-actions
Copy link

⚠️ Changes detected in model-related files

This PR contains changes to files that affect the model layer and unit tests. Before merging, please ensure:

  1. Run the unit tests locally:

    npm run test
  2. Update tests if necessary

@github-actions
Copy link

⚠️ Changes detected in model-related files

This PR contains changes to files that affect the model layer and unit tests. Before merging, please ensure:

  1. Run the unit tests locally:

    npm run test
  2. Update tests if necessary

@Bhoomika2905 Bhoomika2905 marked this pull request as ready for review November 30, 2025 09:07
@nk1408
Copy link
Collaborator

nk1408 commented Dec 4, 2025

In context:
Lines 132-205: stepTrace() method — architectural violation
Problem: Contains business logic in Core Model
Fix: Move orchestration to NavigationService; Context only manages the stack

src/model/factory.ts
This is just factory method for creating processor class, should pass all layers data into it

There are quite lot of conditionals in boxplot. Would extending that and creating new Violin Boxplot look good?

Please resolve merge conflicts after refactoring.

Thanks

@github-actions
Copy link

github-actions bot commented Dec 9, 2025

⚠️ Changes detected in model-related files

This PR contains changes to files that affect the model layer and unit tests. Before merging, please ensure:

  1. Run the unit tests locally:

    npm run test
  2. Update tests if necessary

@github-actions
Copy link

github-actions bot commented Dec 9, 2025

⚠️ Changes detected in model-related files

This PR contains changes to files that affect the model layer and unit tests. Before merging, please ensure:

  1. Run the unit tests locally:

    npm run test
  2. Update tests if necessary

@Bhoomika2905
Copy link
Collaborator Author

I have addressed the comments and solved the merge conflict.

@nk1408 nk1408 changed the title Feat: support violin plot feat: support violin plot Dec 9, 2025
@nk1408 nk1408 merged commit d68343f into main Dec 9, 2025
7 checks passed
@nk1408 nk1408 deleted the violin_plot_maidr branch December 9, 2025 16:35
@xabilitylab
Copy link
Collaborator

🎉 This PR is included in version 3.39.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@xabilitylab xabilitylab added the released For issues/features released label Dec 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

released For issues/features released

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants