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(Callout): visual modernisation #5834

Merged
merged 5 commits into from
Jan 10, 2023

Conversation

CPerinet
Copy link
Contributor

@CPerinet CPerinet commented Jan 10, 2023

Proposition for modernisation of callouts. Reduce unnecessary visual drag while retaining prominence.

  • More consistent and slightly increased spacings
  • Reduce header size from h4 to h5
  • Reduce icon size from 20px to 16px
  • Use intent colour as default text colour.
Before After
Screenshot 2023-01-10 at 12 08 42 Screenshot 2023-01-10 at 12 08 23
Screenshot 2023-01-10 at 12 11 02 Screenshot 2023-01-10 at 12 10 51

Accessibility checks

Given background colours uses opacity, I chose to evaluate accessibility with the callout on a $white / $light-gray5 background for light theme and $dark-gray1 / $dark-gray2 for dark theme.

Theme Intent WCAG Normal Text
Light Default AA
Light Primary AA
Light Success AA
Light Warning AA
Light Danger AA
Dark Default AA
Dark Primary AA
Dark Success AA
Dark Warning AA
Dark Danger AA

@@ -8,7 +8,7 @@ Callout

Markup:
<div class="#{$ns}-callout {{.modifier}}">
<h4 class="#{$ns}-heading">Callout Heading</h4>
<h5 class="#{$ns}-heading">Callout Heading</h5>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

In line with the dialog update, reducing the heading level to fit better in dense UIs.

@@ -25,20 +25,20 @@ Styleguide callout
@include running-typography();
background-color: rgba($gray3, 0.15);
border-radius: $pt-border-radius;
padding: $pt-grid-size ($pt-grid-size * 1.2) ($pt-grid-size * 0.9);
padding: $pt-grid-size * 1.5;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

  • Make the spacing consistent all around.
  • Increase inner spacing.

@@ -93,6 +97,7 @@ Styleguide callout

.#{$ns}-dark & {
background-color: rgba($color, 0.2);
color: map-get($pt-dark-intent-text-colors, $intent);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Make default text colour intent. Adding accessibility checks

@CPerinet CPerinet marked this pull request as ready for review January 10, 2023 13:23
@adidahiya adidahiya changed the title Callout modernisation [core] feat(Callout): visual modernisation Jan 10, 2023
@adidahiya
Copy link
Contributor

adidahiya commented Jan 10, 2023

Color changes LGTM. I also tested contrast without the docs example container background (by default, it increases the apparent contrast), because I think $pt-app-background-color and $pt-dark-app-background-color are more representative of real UI scenarios:

image

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.

None yet

2 participants