Skip to content
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.

Sign up for GitHub

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

Jump to bottom

fix: resolve React warnings and DOM nesting issues #1763

Open
wants to merge 14 commits into
base: main
Choose a base branch
from
Open

fix: resolve React warnings and DOM nesting issues #1763

wants to merge 14 commits into from
+81 −87

Conversation

hexart
Copy link
Contributor

@hexart hexart commented Jan 21, 2025 •
edited
Loading

Description

This PR fixes multiple React warnings and DOM nesting issues across several components:

  • Prevent state updates during render in ChatProfiles component
  • Fix invalid DOM nesting of buttons in ThreadOptions and ThreadList
  • Fix invalid DOM nesting of <h2> and <p> tags in Markdown component
  • Add proper key prop to React Fragments in Messages component

Fixes

  1. Fixed React Component State Updates:

    • Resolved state updates during render in ChatProfiles component
    • Moved setChatProfile calls into useEffect hook
    • Added proper null checks for config object
    • Added optional chaining for safer config access

  2. Fixed DOM Nesting Issues:

    • Resolved invalid button nesting in ThreadOptions and ThreadList
      • Replaced nested Button with properly styled DropdownMenuTrigger
      • Restructured component layout with relative positioning
      • Maintained original styling and functionality
    • Resolved invalid DOM nesting of heading elements in Markdown component
      • Implemented proper handling of block elements inside paragraphs
      • Added logic to switch to div container when needed

  3. Fixed Test Cases and Lint Errors:

    • Updated command test to handle whitespace properly
    • Fixed ESLint warnings
      • Added ESLint disable directives for valid table component imports
      • Removed unused 'size' import from ChatProfiles

Testing Done

  • Confirmed browser console is clean with no error messages
  • Tested chat profile selection functionality
  • Confirmed thread options and list still work as expected
  • Validated markdown content renders correctly

Types of changes

  • Bug fix (non-breaking change which fixes warnings/issues)
  • No new feature added

hexart added 7 commits January 21, 2025 10:54
@hexart
fix: prevent state updates during render in App component
2f0156e 
- Move chatProfile selection logic into useEffect
- Add null checks for config object
- Fix React warning about setState during render"
@hexart
fix: resolve invalid DOM nesting of h2 in p tags in Markdown.tsx
a00ed5a 
- Update Markdown component to handle heading elements properly
- Fix HTML validation warnings"
@hexart
fix: replace p tags with div in message content
1c70624 
- Fix DOM nesting validation warnings
- Maintain existing styling and layout"
@hexart
fix: add key to fragment in Messages component
7af4ca8 
- Add key to React.Fragment in message map function
- Fix React warning about missing keys in list
@hexart
fix: remove nested button in thread options
3fea378 
- Replace Button component with styled DropdownMenuTrigger
- Fix DOM nesting warning for button elements
@hexart
fix: resolve button nesting in thread list
af44de3 
- Move ThreadOptions outside of SidebarMenuButton
- Fix DOM nesting warnings
@hexart
fix: handle undefined checks in ChatProfiles
6643e10 
- Add proper optional chaining for config access
- Move setChatProfile call to useEffect
- Fix TypeScript warnings about potential undefined values
@dosubot dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. frontend Pertains to the frontend. labels Jan 21, 2025
hexart and others added 5 commits January 21, 2025 13:05
@hexart
test: update command test to handle whitespace
8f1ae0c 
- Make text comparison more resilient by using trim()
- Focus on functional correctness rather than exact formatting
- Maintain existing test coverage while improving reliability"
@hexart
fix: resolve lint errors
15d479a 
- Remove unused import 'size' from ChatProfiles
@hexart
fix: add specific eslint disable directives for table components
018a912 
- Add eslint-disable-next-line for Card and Table imports
- Fix lint errors while preserving markdown table functionality
- Ensure ESLint correctly recognizes component usage in table renderer
@hexart
Merge branch 'main' into fix/app-render-warning
4941faa
@dokterbob
Merge branch 'main' into fix/app-render-warning
4936a0c
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@willydouhard willydouhard Awaiting requested review from willydouhard

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
frontend Pertains to the frontend. size:L This PR changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@hexart @dokterbob