fix: implement 7 UI fixes for editor interface (#2488) #2489
Conversation
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
- Fix flexbox inputs dropdown width condensation - Reduce gap in display dropdown from space-y-2.5 to space-y-2 with px-4 py-4 - Remove tooltip arrow and add mt-1 spacing for history button - Vertically center icons with text in subscriptions page - Apply text-foreground-primary styling for active states in toolbar buttons - Update border thickness slider with custom increments (0-16px) - Update corner radius slider with custom increments and Tailwind class mapping Closes #2488 Co-Authored-By: [email protected] <[email protected]>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
| } | ||
|
|
||
| const BORDER_RADIUS_INCREMENTS = [0, 0.25, 0.5, 0.75, 1, 1.5, 2, 2.5, 3, 3.5, 4, 4.5, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16]; | ||
|
|
||
| const TAILWIND_RADIUS_MAP: Record<number, string> = { |
There was a problem hiding this comment.
TAILWIND_RADIUS_MAP is defined but not used; implement mapping logic when useTailwindClasses is true.
… dropdown width - Add STANDARD_INCREMENTS constant with exact values: 0, 0.25, 0.5, 0.75, 1, 1.5, 2, 2.5, 3, 3.5, 4, 4.5, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16 - Update padding and margin InputRange components to use customIncrements prop - Consolidate increment arrays across border, radius, padding, and margin components - Increase flexbox dropdown minimum width from 200px to 240px - Maintain existing functionality while improving slider precision Addresses feedback from GitHub issue #2488 Co-Authored-By: [email protected] <[email protected]>
…tion - Modify handleBoxChange to use consolidated properties (borderWidth, padding, margin, borderRadius) when updating all sides - This ensures Tailwind generates consolidated classes like border-[3px] instead of individual directional classes - Fixes issue where border-t-[3px] border-r-[3px] border-b-[3px] border-l-[3px] was generated instead of border-[3px] - Individual side changes still use specific properties for targeted updates Addresses core Tailwind class consolidation requirement from GitHub issue #2488 Co-Authored-By: [email protected] <[email protected]>
|
Closing due to inactivity for more than 7 days. Configure here. |
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
| 12: 'rounded-xl', | ||
| 16: 'rounded-2xl', | ||
| }; | ||
|
|
There was a problem hiding this comment.
InputRange now supports customIncrements and snapping, which is good. Note that the useTailwindClasses prop is accepted but TAILWIND_RADIUS_MAP isn’t used; if the intent is to map certain values to Tailwind classes, additional logic is needed.
@Kitenite may need you to oversee this – devin seems to be trying to make another stab so maybe take a look after? |
- Update local boxState after CSS changes in useBoxControl to fix slider/toolbar synchronization - Add hideArrow prop to version-history tooltip in top-bar - Addresses GitHub feedback from PR #2489 Co-Authored-By: [email protected] <[email protected]>
- Add immediate onChange handler for instant UI feedback in InputRange - Update local boxState immediately before CSS updates in useBoxControl - Reduce debounce delay from 500ms to 300ms for better responsiveness - Add setTimeout for CSS updates to batch changes and prevent conflicts - Ensure toolbar display stays in sync with slider values during interaction Co-Authored-By: [email protected] <[email protected]>
| } else { | ||
| editorEngine.style.update(property, cssValue); | ||
| } | ||
| }, 50); |
There was a problem hiding this comment.
Using setTimeout with a fixed 50ms delay risks race conditions when rapid updates occur. The callback captures a potentially stale 'currentState'. Consider a debounced or cancelable approach to ensure CSS updates use the latest state.
- Remove problematic setTimeout from handleBoxChange to prevent race conditions - Update CSS first, then local state to ensure proper synchronization - Remove immediateOnChange handler that was causing double updates - Reduce debounce delay to 150ms for better responsiveness - Addresses GitHub feedback about slider/top-bar number desynchronization Co-Authored-By: [email protected] <[email protected]>
| updates.set(property, cssValue); | ||
|
|
||
|
|
||
| // Update CSS first to ensure DOM reflects the change |
There was a problem hiding this comment.
CSS is updated immediately before state update (removed setTimeout). Consider adding error handling around editorEngine.style.update to ensure local state stays in sync if the CSS update fails.
- Replace debounced onChange with immediate handleValueChange callback - Eliminate 150ms delay causing lag and jumping behavior - Ensure slider, input box, and top-bar numbers sync instantly - Addresses user feedback on slider synchronization issues Co-Authored-By: [email protected] <[email protected]>
- Add missing useCallback import to fix Vercel deployment failure - Remove conflicting useEffect that caused slider jumping behavior - Ensure instant synchronization between slider, input, and top-bar - Addresses user feedback on unacceptable lag and jumping issues Co-Authored-By: [email protected] <[email protected]>
|
|
||
| // Cleanup debounce on unmount | ||
| useEffect(() => { |
There was a problem hiding this comment.
The useEffect now runs only once on mount (empty dependency array). Without 'value' in dependencies, changes to the prop won’t update localValue. Verify if this is the intended behavior for syncing external updates.
Description
This PR addresses three specific issues identified in GitHub feedback on the original PR #2489:
Fixed slider/toolbar value synchronization: Updated the
handleBoxChangefunction inuseBoxControlhook to update localboxStateafter CSS changes, ensuring slider values and toolbar display stay perfectly synchronized.Removed tooltip arrow from version-history: Added
hideArrowprop to the version-history tooltip in the top-bar component, consistent with other tooltips in the interface.Verified standard increments implementation: Confirmed that all slider components (border, radius, padding, margin) properly use the
STANDARD_INCREMENTSarray with values: 0, 0.25, 0.5, 0.75, 1, 1.5, 2, 2.5, 3, 3.5, 4, 4.5, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16.Root Cause Analysis
The synchronization issue was caused by the
useBoxControlhook updating CSS viaeditorEngine.style.update()but not updating the localboxStatethat toolbar components use for display. This created a disconnect where slider changes updated styles but the UI showed stale values.Related Issues
Addresses GitHub feedback from @drfarrell on PR #2489
Type of Change
Testing
Due to missing Supabase environment variables, local testing was not possible. Changes have been validated through:
bun formatManual Testing Required:
Key Areas for Review
use-box-control.ts- verify thesetBoxStatecall doesn't cause infinite re-renders or performance issuesboxStateupdates don't negatively affect other UI componentsAdditional Notes
Important
Fixes UI synchronization issues, removes tooltip arrow, and verifies slider increments in editor interface.
useBoxControlby updatingboxStateafter CSS changes.top-bar/index.tsx.STANDARD_INCREMENTSinborder.tsx,margin.tsx,padding.tsx,radius.tsx.hideArrowprop toTooltipContentintop-bar/index.tsx.InputRangeininput-range.tsxto handle custom increments and Tailwind classes.handleBoxChangeinuse-box-control.tsto update local state after CSS updates.display/index.tsxandchat-tab/history.tsx.This description was created by
for d88850b. You can customize this summary. It will automatically update as commits are pushed.