[pull] main from commontoolsinc:main #138
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ts (#2152) * Fix ct-card empty section detection with JS-based slot monitoring ## Problem ct-card was showing excessive vertical whitespace when header/footer slots were empty. The previous CSS-only fix (commit 7696b91) using `:not(:has(*))` appeared to work in testing but failed in practice. ## Root Cause The `:not(:has(*))` selector cannot distinguish between: 1. A slot with no assigned content (should hide) 2. A slot showing fallback content (the slot always has children) Even when nothing is slotted, the `<slot name="header">` contains fallback child elements (card-title-wrapper div, nested slots), making `:has(*)` always return true. Additionally, for content detection, we were only checking the named `slot[name="content"]` but missing the default `<slot></slot>` inside it, which is where content goes when using `<ct-card><content></ct-card>`. ## Solution Implemented JavaScript-based slot detection: 1. Listen to `slotchange` events on all slots 2. Check `assignedNodes()` to detect actual slotted content 3. Add/remove `.empty` class which CSS uses to hide sections 4. For content, check BOTH named slot AND default slot ## Testing Verified with food-recipe pattern in Playwright: - Empty header/footer: `display: none` ✓ - Content visible: `display: block` ✓ - Spacing: 25px on all sides (1px border + 24px padding) ✓ - Top/bottom now matches left/right ✓ Before: 49px top, 73px bottom (asymmetric) After: 25px all sides (symmetric) Generated with [Claude Code](https://claude.com/claude-code) via [Happy](https://happy.engineering) Co-Authored-By: Claude <noreply@anthropic.com> Co-Authored-By: Happy <yesreply@happy.engineering> * Fix ct-card title/description/action slots not displaying The CSS rule was trying to hide the wrapper when empty, but can't detect slot assignment. The slotted elements (with slot="title", etc.) are in the light DOM, not in the shadow DOM where the wrapper lives. Solution: Use same JS-based detection approach for title-wrapper visibility. Check title/action/description slots with assignedNodes() and toggle .empty class on both the title-wrapper and header elements. This fixes Test 5 in the comprehensive test suite where title/description/ action slots were present but not displaying. Generated with [Claude Code](https://claude.ai/code) via [Happy](https://happy.engineering) Co-Authored-By: Claude <noreply@anthropic.com> Co-Authored-By: Happy <yesreply@happy.engineering> * Fix ct-card whitespace detection and header-only spacing Addresses cubic's PR feedback about whitespace-only text nodes being counted as content. Changes: - Add hasContent() helper that filters whitespace-only text nodes and empty elements - Apply filtering consistently to all slot checks (header, content, footer, title, action, description) - Fix vertical spacing for header-only cards by adding bottom padding when content section is empty This prevents formatting whitespace in HTML from incorrectly showing empty card sections. 🤖 Generated with [Claude Code](https://claude.com/claude-code) via [Happy](https://happy.engineering) Co-Authored-By: Claude <noreply@anthropic.com> Co-Authored-By: Happy <yesreply@happy.engineering> * Fix hasContent to not hide childless elements Addresses cubic's feedback that elements with no children (like <img>, <ct-icon>, or custom elements) were incorrectly being treated as empty. Changed logic: - Elements with NO children are now considered content (they're self-contained like images, icons, etc.) - Elements WITH children are only empty if all children are whitespace This preserves the whitespace filtering while correctly handling icon/image/custom elements. 🤖 Generated with [Claude Code](https://claude.com/claude-code) via [Happy](https://happy.engineering) Co-Authored-By: Claude <noreply@anthropic.com> Co-Authored-By: Happy <yesreply@happy.engineering> * Refactor slot content detection into shared utility Extract the finicky slot content detection logic into a reusable utility function that can be used by other components. Changes: - Add packages/ui/src/v2/utils/slots.ts with nodeHasContent() and slotHasContent() functions - Replace inline hasContent logic in ct-card with slotHasContent() - Export slots utility from utils/index.ts This makes the logic: - Reusable across components that need slot content detection - Easier to test in isolation - Easier to maintain (single source of truth) - Better documented with JSDoc 🤖 Generated with [Claude Code](https://claude.com/claude-code) via [Happy](https://happy.engineering) Co-Authored-By: Claude <noreply@anthropic.com> Co-Authored-By: Happy <yesreply@happy.engineering> * Fix slot content detection for elements with only non-text children The nodeHasContent() function now correctly handles elements that contain only non-text child elements (icons, buttons, images, etc.) by checking element.children.length before falling back to textContent checks. Previously, elements like `<div><ct-icon name="star"></ct-icon></div>` would be incorrectly marked as empty because textContent would be empty. This addresses cubic's feedback on PR #2152. Generated with [Claude Code](https://claude.ai/code) via [Happy](https://happy.engineering) Co-Authored-By: Claude <noreply@anthropic.com> Co-Authored-By: Happy <yesreply@happy.engineering> * Reduce to minimal fix * Format pass --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Happy <yesreply@happy.engineering> Co-authored-by: Ben Follington <5009316+bfollington@users.noreply.github.com>
* feat(ui): Add ct-file-input base component Create generic file upload component that can handle any file type. Features: - Accepts any file type via 'accept' attribute - Smart preview rendering based on MIME type (images vs documents) - Protected methods for subclass extension (processFile, renderPreview, etc.) - No compression logic in base class - Cell-based reactive data binding This component serves as the base for ct-image-input and enables PDF/document upload use cases. Generated with [Claude Code](https://claude.ai/code) via [Happy](https://happy.engineering) Co-Authored-By: Claude <noreply@anthropic.com> Co-Authored-By: Happy <yesreply@happy.engineering> * refactor(ui): Refactor ct-image-input to extend ct-file-input Convert ct-image-input from standalone component to subclass of ct-file-input. Changes: - Extends CTFileInput instead of BaseElement - ImageData now extends FileData - Compression logic moved to override methods - Image dimension extraction in processFile override - Capture attribute added via renderFileInput override - Backward compatibility maintained via property aliases (images/maxImages) - Event details include both 'images' and 'files' properties This reduces code duplication and establishes clear inheritance hierarchy while maintaining full backward compatibility with existing code. Generated with [Claude Code](https://claude.ai/code) via [Happy](https://happy.engineering) Co-Authored-By: Claude <noreply@anthropic.com> Co-Authored-By: Happy <yesreply@happy.engineering> * test(ui): Add manual test file for ct-file-input and ct-image-input Create HTML test page to verify: - ct-file-input with PDF support - ct-file-input with mixed image/PDF support - ct-image-input backward compatibility (images property) - ct-image-input camera capture support Test file includes event listeners to verify both 'images' and 'files' properties are present for backward compatibility. Generated with [Claude Code](https://claude.ai/code) via [Happy](https://happy.engineering) Co-Authored-By: Claude <noreply@anthropic.com> Co-Authored-By: Happy <yesreply@happy.engineering> * feat(types): Add JSX types for ct-file-input component Add TypeScript JSX definitions for ct-file-input: - CTFileInputElement interface - CTFileInputAttributes interface with all properties - IntrinsicElements entry for JSX support This enables type-safe usage of ct-file-input in patterns. Generated with [Claude Code](https://claude.ai/code) via [Happy](https://happy.engineering) Co-Authored-By: Claude <noreply@anthropic.com> Co-Authored-By: Happy <yesreply@happy.engineering> * fix: Address cubic code review issues 1. **maxSizeBytes now used in ct-file-input**: Added console.warn when files exceed size threshold 2. **max-file guard fixed**: Only enforces limit in multiple mode, allowing single-file replacement 3. **Removed test-file-input.html**: Browser cannot load TypeScript directly; test-file-upload.tsx pattern serves this purpose 4. **Fixed ct-image-input handler conflict**: Made parent _handleFileChange protected, subclass calls via super 5. **Fixed event recursion**: Override emit() method to add backward-compat properties instead of listening/re-emitting These changes ensure proper file validation, fix single-file selection replacement, and eliminate infinite recursion in event handling. Generated with [Claude Code](https://claude.ai/code) via [Happy](https://happy.engineering) Co-Authored-By: Claude <noreply@anthropic.com> Co-Authored-By: Happy <yesreply@happy.engineering> * fix: Address additional cubic code review issues - Move maxSizeBytes check to after compression so it validates the final file size - Fix max file guard to only apply in multiple mode, allowing single-file replacement - Note: test-file-input.html import issue is moot as file was removed in earlier commit Generated with [Claude Code](https://claude.ai/code) via [Happy](https://happy.engineering) Co-Authored-By: Claude <noreply@anthropic.com> Co-Authored-By: Happy <yesreply@happy.engineering> * Remove test files, screenshots, and bug reports from PR These files were accidentally included and are not part of the ct-file-input/ct-image-input PDF support feature: - 76 playwright screenshot files - 8 bug report and documentation files - 11 test pattern files - package.json/package-lock.json (playwright test dependency) This reduces the PR from 6,656 lines to just the actual feature code. Generated with [Claude Code](https://claude.ai/code) via [Happy](https://happy.engineering) Co-Authored-By: Claude <noreply@anthropic.com> Co-Authored-By: Happy <yesreply@happy.engineering> * Format and lint --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Happy <yesreply@happy.engineering> Co-authored-by: Ben Follington <5009316+bfollington@users.noreply.github.com>
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.4)
Can you help keep this open source service alive? 💖 Please sponsor : )