-
Notifications
You must be signed in to change notification settings - Fork 9
Remove use of Children.map in favor of alternative approach with restored highlighting support #96
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
|
||
| it('supports render prop for custom highlighting', async () => { | ||
| const testText = 'Test text for highlighting' | ||
| const mockRender = jest.fn((params) => params.children) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot there is a lint error here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed the TypeScript lint error by properly typing the Jest mock function for the render prop. The mock function now uses explicit TypeScript generics and parameter typing to satisfy the linter requirements. Commit eb00c92.
| expect(result.current.spokenText).toBe(testText) | ||
| expect(mockRender).toHaveBeenCalledWith({ | ||
| children: testText, | ||
| boundary: expect.any(Object), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another lint error.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot this lint error needs to be fixed too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed the TypeScript lint error by replacing unsafe expect.any() calls with direct type checking using typeof. The test now verifies the boundary object structure without triggering unsafe assignment errors. Commit 775a80c.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #96 +/- ##
==========================================
- Coverage 98.37% 96.62% -1.75%
==========================================
Files 7 7
Lines 614 652 +38
Branches 158 173 +15
==========================================
+ Hits 604 630 +26
- Misses 10 22 +12 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| return currentChild | ||
| }) | ||
| // For complex content, return as-is (highlighting would need custom render prop) | ||
| return children |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot this breaks the functionality in component.tsx by no longer highlighting text when using markTextAsSpoken. Add a default implementation of the render prop in component.tsx that preserves highlighting text when markTextAsSpoken is true.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've added a default render prop implementation in component.tsx that restores highlighting functionality for complex React children when markTextAsSpoken is true. The implementation recursively processes React elements similar to the old parseChildrenRecursively function, ensuring that highlighting works properly with nested JSX content. All tests pass and the functionality is now restored. Commit 269f727.
| volume, | ||
| children, | ||
| text, | ||
| render, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot can you add a render.story.tsx in packages/story that demonstrates how to use the render prop with the component.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've added a comprehensive render.story.tsx file in packages/story that demonstrates various ways to use the render prop with the TextToSpeech component. The story includes 6 different examples:
- BasicRenderProp - Gradient background highlighting
- AnimatedRenderProp - Pulsing animated effects
- BorderStyleRenderProp - Custom border styling
- ComplexContentRenderProp - Works with complex JSX content
- ConditionalRenderProp - Different styles based on word length
- MultiColorRenderProp - Cycles through different colors
Each example shows different highlighting techniques and demonstrates the flexibility of the render prop pattern for custom text highlighting during speech synthesis. The story builds successfully and all linting/tests pass.
Commit f7bc567.
|
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. Warning Firewall rules blocked me from connecting to one or more addressesI tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
Co-authored-by: morganney <848178+morganney@users.noreply.github.com>
Co-authored-by: morganney <848178+morganney@users.noreply.github.com>
Co-authored-by: morganney <848178+morganney@users.noreply.github.com>
… with type checking Co-authored-by: morganney <848178+morganney@users.noreply.github.com>
…mplex content Co-authored-by: morganney <848178+morganney@users.noreply.github.com>
Co-authored-by: morganney <848178+morganney@users.noreply.github.com>
f7bc567 to
727413d
Compare
This PR removes the legacy
Children.mapusage from the tts-react library and replaces it with modern React patterns, following the guidance from React's documentation on Children.map alternatives.Changes Made
Core Implementation
Children.mapdependency: Replaced theparseChildrenRecursivelyfunction that relied onChildren.mapwith a newcreateHighlightedContentfunction that uses manual React element inspectionextractTextFromChildrenfunction that traverses React children without using legacy Children APIsComponent Enhancement
TextToSpeechcomponent now provides a default render prop implementation that preserves highlighting functionality for complex React children whenmarkTextAsSpokenis trueChildren.mapapproach, ensuring that highlighting works properly with nested JSX contentAPI Enhancements
The hook now supports multiple input patterns while maintaining backward compatibility:
Technical Benefits
markTextAsSpokenTesting
The library now follows React best practices while providing the same functionality plus additional flexibility for advanced use cases, with full highlighting support restored for complex React children.
Fixes #57.
💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.