fix(ui-color-picker): fix popover scrolling + docs routing#2237
Merged
fix(ui-color-picker): fix popover scrolling + docs routing#2237
Conversation
|
balzss
force-pushed
the
fix/color-picker-scrolling
branch
2 times, most recently
from
9cd5c36 to
31f7770
Compare
Added historyApiFallback to webpack dev server config to properly handle client-side routing. Without this, refreshing the page on non-root routes would return 404 errors. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
…port This fix ensures the ColorPicker popover displays properly and is scrollable when its content (ColorMixer, ColorPreset, ColorContrast) would exceed the available viewport space. Key improvements: - Calculate max height dynamically based on available viewport space - Support both upward and downward popover placement - Use double requestAnimationFrame to ensure accurate measurements after Emotion finishes injecting CSS-in-JS styles and child components complete their mount lifecycle - Add opacity transition to prevent visual jump when popover opens - Reset calculated height state on close to fix alternating open/close behavior - Add visual regression tests The double rAF approach was necessary because a single requestAnimationFrame was insufficient for the timing requirements of Emotion's dynamic style injection. The solution works reliably regardless of React StrictMode setting. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
balzss
force-pushed
the
fix/color-picker-scrolling
branch
from
31f7770 to
9e06ba2
Compare
ToMESSKa
requested changes
Nov 13, 2025
Contributor
ToMESSKa
left a comment
ToMESSKa
left a comment
There was a problem hiding this comment.
If I open the ColorPicker, select a color and click 'Add' and then open the ColorPicker again, the viewport size is not respected and scrolling isn't avaliable.
HerrTopi
approved these changes
Nov 14, 2025
Contributor
HerrTopi
left a comment
HerrTopi
left a comment
There was a problem hiding this comment.
Lgtm. At least good enough. If you select a color and close the popover, it wont scroll if opened again. If closed and open again, it works as expected. Good for now
balzss
force-pushed
the
fix/color-picker-scrolling
branch
from
72150d8 to
73c4c6b
Compare
HerrTopi
approved these changes
Nov 18, 2025
balzss
force-pushed
the
fix/color-picker-scrolling
branch
from
73c4c6b to
64c75dc
Compare
ToMESSKa
approved these changes
Nov 18, 2025
matyasf
requested changes
Nov 18, 2025
Collaborator
matyasf
left a comment
matyasf
left a comment
There was a problem hiding this comment.
There is a horizontal scrollbar in FF, otherwise it looks good
Comment on lines
+148
to
+154
| cy.injectAxe() | ||
| // TODO: Fix ARIA violations before enabling a11y check | ||
| // - aria-allowed-attr (critical): 1 node | ||
| // - aria-prohibited-attr (serious): 3 nodes | ||
| // ColorMixer has aria-disabled on plain div without proper role | ||
| // cy.checkA11y('.axe-test', axeOptions, terminalLog) | ||
| }) |
Collaborator
There was a problem hiding this comment.
These issues existed before this PR, right? (I dont see them when I run in the example page with a ColorPicker open)
Contributor
Author
There was a problem hiding this comment.
i think they existed but we didn't have colorpicker examples before this pr
Fixed two issues with the ColorPicker mixer button: 1. Alignment calculation returning 0 due to timing - measurement now happens after CSS-in-JS styles are applied using requestAnimationFrame 2. Visual jump on load - button now starts bottom-aligned and switches to top-aligned after calculation completes, minimizing the jump 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
balzss
force-pushed
the
fix/color-picker-scrolling
branch
from
64c75dc to
32ddef6
Compare
Contributor
Author
@matyasf this should be fixed now |
matyasf
approved these changes
Nov 19, 2025
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 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.
Summary
Fixed multiple ColorPicker issues related to popover scrolling and mixer button alignment.
Issue 1: Popover Scrolling
Fixed ColorPicker popover scrolling when content exceeds viewport height. The popover now dynamically calculates available space based on placement (above/below trigger) and becomes scrollable when needed. A smooth fade-in transition prevents visual jump on opening.
Issue 2: Mixer Button Alignment
Fixed mixer button alignment and visual jump issues:
requestAnimationFrameto defer measurement until CSS-in-JS styles are appliedKey Changes
requestAnimationFramefor popover to wait for Emotion CSS-in-JS injection and child component mountingrequestAnimationFramefor mixer button alignment to wait for layout completionalignSelfswitching for mixer button to minimize visual jumpTest Plan
Refs INSTUI-4849
🤖 Generated with Claude Code