feat: add feat 378- MAIDR as react interface, grafana plugin

[aarthy2000]

Pull Request

Description

This pull request adds the feature requested in #378 and provides a plugin to visualize accessible bar charts in grafana (protoype)

Related Issues

#378

Changes Made

The init maidr mechanism is exposed so other applications can use it.
For grafana, it accepts the input from grafana in various format (csv content, raw rows tested for now) and generates the maidr payload, svg.
TODO: Creation of svg should utilize the existing mechanism of py binder.

Notes

Currently, the CSV can be provided as text/ file in Grafana plugin, Bar chart SVG is built by this component: https://github.com/xability/maidr/blob/feat/feat_387/xability-maidr-panel/src/components/MaidrBarChart.tsx

This is not scalable and uniform, we need to use the python maidr package here.
Approach 1: Get the CSV from the plugin -> use py-maidr to construct the SVG -> visualize in Grafana
Method 2 here https://codeop.tech/how-to-run-python-code-in-javascript-a-beginners-guide/ sounds like a possible solution

Screenshots (if applicable)

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.

Additional Notes

[claude]

Pull Request Review - PR #501

Overview

This PR adds significant new functionality:

1. Exposes MAIDR as a React interface - Exports public API for React integration (addresses feat: React interface #378)
2. Adds Grafana plugin - Complete Grafana panel plugin for accessible bar charts
3. NPM publishing improvements - Enhanced release workflow with better error handling

Overall Assessment: This is a substantial PR with 19,231 additions that successfully implements the requested features. However, there are several important concerns that should be addressed before merging.

---

✅ Strengths

1. Clean Public API Exposure (src/index.ts)

• ✅ Well-documented initMaidr function with JSDoc
• ✅ Thoughtful exports for React integration
• ✅ Maintains backward compatibility with existing initialization paths

2. Grafana Plugin Architecture

• ✅ Uses theme-aware colors via useTheme2() - follows Grafana best practices
• ✅ Proper TypeScript typing throughout
• ✅ Clean component separation (SimplePanel → MaidrBarChart)
• ✅ Accessibility-first approach with ARIA labels

3. Enhanced CI/CD

• ✅ Added artifact upload for debugging npm logs (lines 69-75 in release.yml)
• ✅ Environment configuration for NPM_TOKEN

---

⚠️ Critical Issues

1. Incomplete PR Checklist

The PR description shows all checkboxes are unchecked:

• Self-review not completed
• Testing not completed
• Documentation not updated
• No unit tests added

Action Required: Complete all checklist items before merging, per CONTRIBUTING.md guidelines.

---

2. Test Coverage Concerns

Main MAIDR Changes

• ❌ No tests added for the new public API exports
• ❌ No tests for initMaidr as a standalone function
• ⚠️ Breaking change risk: Exporting internal components could lead to unintended API coupling

Recommendation: Add integration tests that:

• Verify initMaidr() works when called programmatically
• Test the exported types and enums
• Ensure backward compatibility

Grafana Plugin Tests (xability-maidr-panel/tests/panel.spec.ts:1)

• ❌ Tests appear to be template/scaffolding code and don't match actual implementation
  • Line 20: References 'simple-panel-circle' - doesn't exist in MaidrBarChart
  • Lines 23-35: Tests "Show series counter" option - this option is not in the plugin
  • Tests expect different panel behavior than what's implemented

Action Required: Rewrite E2E tests to match actual implementation:

// Example of what tests should verify:
- Bar chart renders with data
- MAIDR initialization works
- Keyboard navigation functions
- Accessibility features are present
- Panel options (title, xAxisLabel, yAxisLabel) work correctly

---

3. Code Quality Issues

A. Debug Console Logs Left in Production Code

File: xability-maidr-panel/src/components/SimplePanel.tsx:33-36, 40, 61

console.log('Grafana data series:', series);
console.log('Fields:', series.fields.map(f => ({ name: f.name, type: f.type })));
console.log('Selected xField:', xField?.name, xField?.type);
console.log('Selected yField:', yField?.name, yField?.type);
console.log('Missing fields - xField:', !!xField, 'yField:', !!yField);
console.log('Generated bar data:', points);

Issue: These should be removed or converted to proper logging
Action: Remove debug logs or use Grafana's logging utilities

---

B. Dependency on Relative Package Path

File: xability-maidr-panel/package.json:75

"maidr": "file:.."

Issue: This local file dependency won't work when the plugin is published/distributed
Action Required:

• Change to proper npm package reference once MAIDR is published
• Document this limitation in the README
• Consider adding a build step to handle this

---

C. Hardcoded Magic Numbers

File: xability-maidr-panel/src/components/MaidrBarChart.tsx:42, 46, 99

const maidrTextHeight = 40;  // line 42
const margin = { top: 40, right: 20, bottom: 60, left: 60 };  // line 46
setTimeout(() => { ... }, 100);  // line 99

Recommendation: Extract to constants with descriptive names:

const MAIDR_TEXT_DISPLAY_HEIGHT = 40;
const CHART_MARGINS = { top: 40, right: 20, bottom: 60, left: 60 };
const DOM_READY_DELAY_MS = 100;

---

D. ESM Dependency Issue Risk

File: xability-maidr-panel/useEffect dependency array:137

useEffect(() => {
  // ...
}, [data, maidrConfig, chartId, textColor, height, backgroundColor]);

Issue: maidrConfig is an object created inline, causing unnecessary re-renders
Fix: Use useMemo for maidrConfig or move dependencies to primitives

---

4. Architecture Concerns

A. Direct DOM Manipulation in React

File: xability-maidr-panel/src/components/MaidrBarChart.tsx:105-131

The component directly injects <style> tags into the document head from within a React component. This:

• Violates React's declarative philosophy
• Creates potential memory leaks (styles never removed)
• Could cause conflicts in multi-panel dashboards

Recommendation: Use Grafana's emotion/CSS-in-JS approach:

import { css } from '@emotion/css';
import { useStyles2 } from '@grafana/ui';

const getStyles = (theme: GrafanaTheme2, height: number) => ({
  figure: css`
    position: relative;
    height: ${height}px;
  `,
  // ...
});

---

B. Violation of MAIDR Architecture Principles

File: xability-maidr-panel/src/components/MaidrBarChart.tsx:1-261

According to .github/copilot-instructions.md, MAIDR follows strict layered architecture:

• UI → ViewModel → Service → Core Model

Issue: The Grafana plugin bypasses this by:

• Creating SVG manually in UI layer (lines 155-256)
• Building chart data in component (lines 54-66)
• Mixing presentation and data transformation logic

Recommendation:

• Consider creating a proper ViewModel layer
• Move chart rendering logic to a service
• Or document why this violation is acceptable for the plugin context

---

5. Potential Runtime Issues

A. Race Condition in Initialization

File: xability-maidr-panel/src/components/MaidrBarChart.tsx:96-137

if (svgRef.current && !initializedRef.current && data.length > 0) {
  const timer = setTimeout(() => {
    if (svgRef.current) {
      initMaidr(maidrConfig, svgRef.current);

Issues:

1. 100ms delay is arbitrary - may be too short/long depending on browser
2. No cleanup if component unmounts during delay
3. initializedRef prevents re-initialization even if data changes

Fix: Already has cleanup (return () => clearTimeout(timer)), but consider using useLayoutEffect instead of setTimeout

---

B. Missing Error Boundaries

No error boundaries around MAIDR initialization. If initMaidr throws, the entire Grafana panel crashes.

Recommendation: Add error boundary or try-catch:

try {
  initMaidr(maidrConfig, svgRef.current);
} catch (error) {
  console.error('Failed to initialize MAIDR:', error);
  // Show user-friendly error message
}

---

6. Documentation Gaps

Missing Documentation

1. ❌ No README update in main maidr project explaining React API usage
2. ❌ No examples of how to use initMaidr programmatically
3. ❌ Grafana plugin README is generic template, doesn't explain MAIDR-specific features
4. ⚠️ PR description says "TODO: Creation of svg should utilize the existing mechanism of py binder" - is this blocking?

Action Required: Add documentation showing:

• How to use MAIDR in a React app
• How to use MAIDR in Grafana
• Example code snippets
• API reference for exported functions/types

---

7. Security Considerations

✅ No major security issues found

Minor notes:

• Dynamic style injection could theoretically be XSS vector, but content is controlled
• No user input sanitization needed (Grafana handles this)
• No credentials or secrets in code

---

📋 Required Changes Before Merge

High Priority

1. ✅ Complete PR checklist - All items must be checked
2. ❌ Fix/Update E2E tests - Tests must match actual implementation
3. ❌ Remove debug console.log statements
4. ❌ Document the file:.. dependency limitation
5. ❌ Add tests for public API exports in main MAIDR

Medium Priority

6. ⚠️ Add error handling around initMaidr calls
7. ⚠️ Fix dependency array issue in useEffect (use useMemo for maidrConfig)
8. ⚠️ Add user documentation for React API and Grafana plugin
9. ⚠️ Resolve TODO about SVG creation mechanism (or remove if not blocking)

Nice to Have

10. Consider refactoring style injection to use emotion/CSS-in-JS
11. Extract magic numbers to named constants
12. Add error boundaries for better fault isolation

---

🎯 Recommendations

Short Term

1. Don't merge until checklist is complete - This is a requirement per CONTRIBUTING.md
2. Fix the test suite - Current tests will fail and don't test actual functionality
3. Clean up debug code - Remove console.log statements
4. Add basic documentation - At minimum, update main README with React API example

Long Term

1. Consider architectural alignment - Document or address why Grafana plugin doesn't follow MAIDR's layered architecture
2. Plan for npm publishing - The file:.. dependency needs a solution before distribution
3. Expand test coverage - Add integration tests for the public API

---

💭 Questions for Author

1. Is the TODO about py binder SVG creation blocking? (PR description line 10)
2. What's the plan for the maidr: "file:.." dependency when publishing the plugin?
3. Why weren't the E2E tests updated to match the actual implementation?
4. Have you manually tested the Grafana plugin with various data sources?
5. Are there plans to support other chart types (line, scatter, etc.) or just bar charts for now?

---

📊 Summary Score

Category	Rating	Notes
Code Quality	6/10	Good structure, but debug logs and architectural concerns
Test Coverage	3/10	Tests don't match implementation
Documentation	4/10	Missing usage examples and API docs
Security	9/10	No significant concerns
Architecture	7/10	Some violations of stated principles
Overall	6/10	Good foundation, needs work before merge

---

✅ Approval Status: Changes Requested

This PR implements valuable functionality but requires the following before approval:

1. Complete PR checklist
2. Fix/rewrite E2E tests
3. Remove debug code
4. Add basic documentation
5. Address the file:.. dependency issue

Great work on the implementation! The core functionality looks solid. With the above changes, this will be ready to merge.

cc: @aarthy2000