Skip to content

Commit b7fbf0b

Browse files
balzssclaude
andcommitted
fix(ui-color-picker): fix popover scrolling and button alignment issues
- Calculate popover max height dynamically to enable scrolling when content exceeds available viewport space - Fix mixer button alignment visual jump on load by deferring measurement until CSS-in-JS styles are applied - Use double requestAnimationFrame to ensure measurements happen after child components mount and styles are injected - Add opacity transition to prevent visual jump when popover opens - Reset calculated height state when popover closes 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
1 parent dfabfa6 commit b7fbf0b

File tree

3 files changed

+105
-10
lines changed

3 files changed

+105
-10
lines changed

packages/ui-color-picker/src/ColorPicker/index.tsx

Lines changed: 94 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,9 @@ class ColorPicker extends Component<ColorPickerProps, ColorPickerState> {
110110
showHelperErrorMessages: false,
111111
openColorPicker: false,
112112
mixedColor: '',
113-
labelHeight: 0
113+
labelHeight: 0,
114+
calculatedPopoverMaxHeight: undefined,
115+
isHeightCalculated: false
114116
}
115117
}
116118

@@ -130,7 +132,19 @@ class ColorPicker extends Component<ColorPickerProps, ColorPickerState> {
130132

131133
handleInputContainerRef = (el: Element | null) => {
132134
this.inputContainerRef = el
133-
this.setLabelHeight()
135+
136+
if (el) {
137+
// Defer measurement until after layout is complete and CSS-in-JS styles are applied
138+
requestAnimationFrame(() => {
139+
this.setLabelHeight()
140+
})
141+
}
142+
}
143+
144+
popoverContentRef: HTMLDivElement | null = null
145+
146+
handlePopoverContentRef = (el: HTMLDivElement | null) => {
147+
this.popoverContentRef = el
134148
}
135149

136150
setLabelHeight = () => {
@@ -143,6 +157,62 @@ class ColorPicker extends Component<ColorPickerProps, ColorPickerState> {
143157
}
144158
}
145159

160+
// Calculate the maximum height the popover can have without extending beyond
161+
// the viewport. This enables scrolling when the ColorPicker's content (all
162+
// color mixing controls, presets, and contrast checker) would otherwise exceed
163+
// the available viewport space. Without this calculation, the popover would
164+
// render off-screen on smaller viewports.
165+
handlePopoverPositioned = (position?: { placement?: string }) => {
166+
if (this.popoverContentRef) {
167+
// Double requestAnimationFrame ensures measurements happen after all child components
168+
// (ColorMixer, ColorPreset, ColorContrast) complete their mount lifecycle and Emotion
169+
// finishes injecting CSS-in-JS styles. A single rAF was insufficient as styles are
170+
// injected dynamically in componentDidMount(). This timing issue only manifested when
171+
// StrictMode was disabled, since StrictMode's double-rendering provided an accidental
172+
// second measurement pass.
173+
requestAnimationFrame(() => {
174+
// First frame: DOM structure is laid out
175+
requestAnimationFrame(() => {
176+
// Second frame: styles injected, child components mounted, dimensions stable
177+
if (!this.popoverContentRef) return
178+
179+
const rect = this.popoverContentRef.getBoundingClientRect()
180+
const viewportHeight = window.innerHeight
181+
182+
// Detect if popover is positioned above (top) or below (bottom) the trigger.
183+
// The Position component provides placement strings like "top center" or "bottom center".
184+
const placement = position?.placement || ''
185+
const isPositionedAbove = placement.startsWith('top')
186+
187+
let availableHeight: number
188+
189+
if (isPositionedAbove) {
190+
// When opening upward: available space is from viewport top to popover bottom.
191+
// This is the space where the popover can expand within the viewport.
192+
availableHeight = rect.top + rect.height - 16
193+
} else {
194+
// When opening downward: available space is from popover top to viewport bottom.
195+
// Subtract a small buffer (16px) for padding/margin.
196+
availableHeight = viewportHeight - rect.top - 16
197+
}
198+
199+
const propMaxHeight = this.props.popoverMaxHeight
200+
let calculatedMaxHeight = `${Math.max(100, availableHeight)}px`
201+
202+
// If prop specifies a maxHeight, respect it as an additional constraint
203+
if (propMaxHeight && propMaxHeight !== '100vh') {
204+
calculatedMaxHeight = propMaxHeight
205+
}
206+
207+
this.setState({
208+
calculatedPopoverMaxHeight: calculatedMaxHeight,
209+
isHeightCalculated: true
210+
})
211+
})
212+
})
213+
}
214+
}
215+
146216
componentDidMount() {
147217
this.props.makeStyles?.({ ...this.state, isSimple: this.isSimple })
148218
this.checkSettings()
@@ -427,7 +497,12 @@ class ColorPicker extends Component<ColorPickerProps, ColorPickerState> {
427497
}
428498
isShowingContent={this.state.openColorPicker}
429499
onShowContent={() => {
430-
this.setState({ openColorPicker: true, mixedColor: this.state.hexCode })
500+
this.setState({
501+
openColorPicker: true,
502+
mixedColor: this.state.hexCode,
503+
calculatedPopoverMaxHeight: undefined,
504+
isHeightCalculated: false
505+
})
431506
}}
432507
onHideContent={() => {
433508
this.setState({ openColorPicker: false })
@@ -438,8 +513,13 @@ class ColorPicker extends Component<ColorPickerProps, ColorPickerState> {
438513
shouldReturnFocus
439514
shouldCloseOnDocumentClick
440515
offsetY="10rem"
516+
onPositioned={this.handlePopoverPositioned}
517+
onPositionChanged={this.handlePopoverPositioned}
441518
>
442-
<div css={this.props.styles?.popoverContentContainer}>
519+
<div
520+
css={this.props.styles?.popoverContentContainer}
521+
ref={this.handlePopoverContentRef}
522+
>
443523
{this.isDefaultPopover
444524
? this.renderDefaultPopoverContent()
445525
: this.renderCustomPopoverContent()}
@@ -470,7 +550,9 @@ class ColorPicker extends Component<ColorPickerProps, ColorPickerState> {
470550
() =>
471551
this.setState({
472552
openColorPicker: false,
473-
mixedColor: this.state.hexCode
553+
mixedColor: this.state.hexCode,
554+
calculatedPopoverMaxHeight: undefined,
555+
isHeightCalculated: false
474556
})
475557
)}
476558
</div>
@@ -576,7 +658,9 @@ class ColorPicker extends Component<ColorPickerProps, ColorPickerState> {
576658
onClick={() =>
577659
this.setState({
578660
openColorPicker: false,
579-
mixedColor: this.state.hexCode
661+
mixedColor: this.state.hexCode,
662+
calculatedPopoverMaxHeight: undefined,
663+
isHeightCalculated: false
580664
})
581665
}
582666
color="secondary"
@@ -639,7 +723,10 @@ class ColorPicker extends Component<ColorPickerProps, ColorPickerState> {
639723
{!this.isSimple && (
640724
<div
641725
css={this.props.styles?.colorMixerButtonContainer}
642-
style={{ paddingTop: this.state.labelHeight }}
726+
style={{
727+
alignSelf: this.state.labelHeight > 0 ? 'flex-start' : 'flex-end',
728+
paddingTop: this.state.labelHeight
729+
}}
643730
>
644731
<div css={this.props.styles?.colorMixerButtonWrapper}>
645732
{this.renderPopover()}

packages/ui-color-picker/src/ColorPicker/props.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -247,6 +247,8 @@ type ColorPickerState = {
247247
openColorPicker: boolean
248248
mixedColor: string
249249
labelHeight: number
250+
calculatedPopoverMaxHeight: string | undefined
251+
isHeightCalculated: boolean
250252
}
251253

252254
type PropKeys = keyof ColorPickerOwnProps

packages/ui-color-picker/src/ColorPicker/styles.ts

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ const generateStyle = (
5454
spacing
5555
} = componentTheme
5656
const { checkContrast, popoverMaxHeight, margin } = props
57-
const { isSimple } = state
57+
const { isSimple, calculatedPopoverMaxHeight } = state
5858

5959
const cssMargin = mapSpacingToShorthand(margin, spacing)
6060
return {
@@ -127,8 +127,14 @@ const generateStyle = (
127127
},
128128
popoverContentContainer: {
129129
label: 'colorPicker__popoverContentContainer',
130-
maxHeight: popoverMaxHeight || '100vh',
131-
overflow: 'auto'
130+
maxHeight: calculatedPopoverMaxHeight || popoverMaxHeight || '100vh',
131+
overflowY: 'auto',
132+
overflowX: 'hidden',
133+
scrollbarGutter: 'stable',
134+
display: 'flex',
135+
flexDirection: 'column',
136+
opacity: state.isHeightCalculated ? 1 : 0,
137+
transition: 'opacity 150ms ease-in'
132138
},
133139
colorMixerButtonWrapper: {
134140
label: 'colorPicker__colorMixerButtonWrapper',

0 commit comments

Comments
 (0)