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

[core] feat: allow dialog bg color to be overridden as Sass variable #5086

Merged

Conversation

dylangrandmont
Copy link
Contributor

Checklist

  • Includes tests
  • Update documentation

Changes proposed in this pull request:

Allow override of dialog background-color using a variable $pt-dialog-background-color

Reviewers should focus on:

  • Naming convention of variable
  • Appropriateness of using a variable here
  • Whether this needs to be documented (it appears that component-level variables like this are not documented elsewhere)

@palantirtech
Copy link
Member

Thanks for your interest in palantir/blueprint, @dylangrandmont! Before we can accept your pull request, you need to sign our contributor license agreement - just visit https://cla.palantir.com/ and follow the instructions. Once you sign, I'll automatically update this pull request.

Copy link
Contributor

@adidahiya adidahiya left a comment

Choose a reason for hiding this comment

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

sorry for the very delayed review here! please let me know if you'd like to continue working on this PR.

I'm open to this code change but have some comments on the implementation.

Note that we'll also need a corresponding dark theme color variable called $dark-dialog-background-color.

@@ -34,6 +34,7 @@ Markup:
Styleguide dialog
*/

$pt-dialog-background-color: $light-gray4 !default;
Copy link
Contributor

Choose a reason for hiding this comment

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

this should match the naming convention of the other dialog variables:

Suggested change
$pt-dialog-background-color: $light-gray4 !default;
$dialog-background-color: $light-gray4 !default;

@adidahiya adidahiya changed the title Allow background color for dialog to be overridden [core] feat: allow dialog bg color to be overridden as Sass variable Jan 8, 2024
@adidahiya adidahiya enabled auto-merge (squash) January 8, 2024 14:20
@adidahiya adidahiya merged commit ea28092 into palantir:develop Jan 8, 2024
12 checks passed
@dylangrandmont
Copy link
Contributor Author

Thanks for taking this over the line @adidahiya

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.

None yet

3 participants