Skip to content

Conversation

git-nandor
Copy link
Contributor

@git-nandor git-nandor commented Sep 9, 2025

When rendering the AI button inside a flex container with a fixed height, the button was being stretched vertically due to the parent’s default align-items: stretch.

Test:
You can verify the fix by trying the example below on the button docs page.
The visual glitch should no longer appear when the AI secondary button is inside a flex parent with a fixed height.

<>
  <View display="block">
    <Button color="ai-primary" renderIcon={IconAiSolid} margin="small">AI Primary</Button>
    <Button color="ai-secondary" renderIcon={IconAiColoredSolid} margin="small">AI Secondary</Button>
    <IconButton color="ai-primary" screenReaderLabel="AI button" margin="small"><IconAiSolid/></IconButton>
    <IconButton color="ai-secondary" screenReaderLabel="AI button"  margin="small"><IconAiColoredSolid/></IconButton>
  </View>
  
  <div style={{height:"100px", display: "flex", marginLeft:"15px", gap: "24px"}}>
    <Button color="ai-primary" renderIcon={IconAiSolid} >AI Primary</Button>
    <Button color="ai-secondary" renderIcon={IconAiColoredSolid} >AI Secondary</Button>
    <IconButton color="ai-primary" screenReaderLabel="AI button" ><IconAiSolid/></IconButton>
    <IconButton color="ai-secondary" screenReaderLabel="AI button" ><IconAiColoredSolid/></IconButton>
    <IconButton color="secondary" screenReaderLabel="AI button" ><IconAiColoredSolid/></IconButton>
  </div>
</>

@git-nandor git-nandor self-assigned this Sep 9, 2025
Copy link

github-actions bot commented Sep 9, 2025

PR Preview Action v1.6.2
Preview removed because the pull request was closed.
2025-09-25 08:53 UTC

@git-nandor git-nandor force-pushed the ai_secondary_button_visual_glitch branch from 664be3b to e385461 Compare September 9, 2025 15:58
@ToMESSKa
Copy link
Contributor

image @git-nandor in some cases, the the focus ring still looks stretched, can you please take a look at this?

Copy link
Contributor

@ToMESSKa ToMESSKa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see my comment

@git-nandor git-nandor force-pushed the ai_secondary_button_visual_glitch branch from e385461 to 99b761c Compare September 11, 2025 10:30
@git-nandor git-nandor requested a review from ToMESSKa September 11, 2025 10:42
@git-nandor git-nandor force-pushed the ai_secondary_button_visual_glitch branch from 99b761c to 9d7abd7 Compare September 11, 2025 10:52
@git-nandor
Copy link
Contributor Author

The vertical stretch caused by flex wasn’t limited to the secondary version, although the visual glitch only appeared there. I moved the fix to baseButton so it now applies everywhere. I also reviewed the other buttons and they look fine to me. Thanks for catching that!

@git-nandor git-nandor force-pushed the ai_secondary_button_visual_glitch branch 3 times, most recently from 0afa131 to fb34459 Compare September 11, 2025 12:38
@git-nandor git-nandor marked this pull request as draft September 12, 2025 08:55
@git-nandor git-nandor force-pushed the ai_secondary_button_visual_glitch branch 3 times, most recently from faa5b58 to df0cad3 Compare September 15, 2025 13:43
@git-nandor git-nandor force-pushed the ai_secondary_button_visual_glitch branch from df0cad3 to 9e88f96 Compare September 15, 2025 14:23
@git-nandor git-nandor marked this pull request as ready for review September 15, 2025 14:50
Copy link
Collaborator

@matyasf matyasf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now components look off in the regression test (check out the Chromatic diff), there are 3 different vertical positions. It looked better in the original version

Copy link
Contributor

@joyenjoyer joyenjoyer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

UI tests show an anomaly, please check.

@git-nandor
Copy link
Contributor Author

git-nandor commented Sep 18, 2025

aibcomp01 The bug is not limited to the AI button. The components may look “off” at first glance, but i think this is their correct alignment.

The visually appealing centered alignment we saw earlier was actually a side effect of the bug. The flex wrapper was automatically stretching its children, and that stretching caused the background and focus ring to render incorrectly on all of our buttons.

Once the stretch was removed as part of the fix, the children reverted to their natural height and aligned accordingly. I believe this is the expected behavior without additional CSS specifications. If users want a different alignment, they can apply their own CSS rules (e.g. align-items on the wrapper or alignment rules on the children).

I wouldn’t enforce any extra default alignment on the buttons themselves. In fact, the lack of extra specifications is what made the bug visible in the first place. If the wrapper or children had been given custom CSS for alignment that overrode the default stretching, the bug would have been masked.

@git-nandor git-nandor requested a review from matyasf September 18, 2025 07:26
Copy link
Collaborator

@matyasf matyasf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good!

@git-nandor git-nandor dismissed joyenjoyer’s stale review September 25, 2025 08:53

Reviewer on holiday.

@git-nandor git-nandor merged commit aff6f65 into master Sep 25, 2025
10 of 11 checks passed
@git-nandor git-nandor deleted the ai_secondary_button_visual_glitch branch September 25, 2025 08:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants