Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Respect icon intent color passed to Dialog #6947

Merged
merged 7 commits into from
Aug 29, 2024

Conversation

ggdouglas
Copy link
Contributor

@ggdouglas ggdouglas commented Aug 23, 2024

Fixes #6824

Checklist

  • [N/A] Includes tests
  • Update documentation

Changes proposed in this pull request:

Fixes an issue where dialog header would always overwrite the intent of an icon passed into the dialog, either though the icon prop:

<Dialog icon={<Icon icon={IconNames.INFO_SIGN} intent={Intent.PRIMARY} ... />}

or embedded in the title:

<Dialog title={<>Info <Icon icon={IconNames.INFO_SIGN} intent={Intent.PRIMARY} /></>} ... />}

Note: Icons with the color property set (e.g. <Icon icon={IconNames.INFO_SIGN} color={Colors.BLUE3} />) are not affected by this bug since they set color on the fill of the icon's SVG directly.

Screenshot

Light Mode
Before After
before-light after-light
Dark Mode
Before After
before-dark after-dark

@ggdouglas ggdouglas force-pushed the gdouglas/fix-dialog-icon-intent branch from 9ede586 to fb4b7ce Compare August 23, 2024 14:34
@svc-palantir-github
Copy link

Add generated changelog entries

Build artifact links for this commit: documentation | landing | table | demo

This is an automated comment from the deploy-preview CircleCI job.

@svc-palantir-github
Copy link

Use qualified selector to omit icons with intent from coloring

Build artifact links for this commit: documentation | landing | table | demo

This is an automated comment from the deploy-preview CircleCI job.

@ggdouglas ggdouglas requested a review from invliD August 23, 2024 15:46
@svc-palantir-github
Copy link

Remove unnecessary prefixed icon class

Build artifact links for this commit: documentation | landing | table | demo

This is an automated comment from the deploy-preview CircleCI job.

@ggdouglas ggdouglas force-pushed the gdouglas/fix-dialog-icon-intent branch from 0627995 to 2d4ae48 Compare August 24, 2024 21:41
@svc-palantir-github
Copy link

Remove more duplicate nested classes

Build artifact links for this commit: documentation | landing | table | demo

This is an automated comment from the deploy-preview CircleCI job.

@ggdouglas ggdouglas merged commit 4250873 into develop Aug 29, 2024
13 checks passed
@ggdouglas ggdouglas deleted the gdouglas/fix-dialog-icon-intent branch August 29, 2024 21:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] Icon intent ignored in <Dialog/> title
4 participants