Skip to content
This repository has been archived by the owner on May 24, 2024. It is now read-only.

Updated redwood theme's default density to 'comfortable' #366

Merged
merged 3 commits into from
Jan 24, 2024

Conversation

artpark
Copy link
Contributor

@artpark artpark commented Jan 23, 2024

Summary

What was changed:

The default density for redwood has been updated to 'comfortable'.

Why it was changed:

Consumers could forget the density option, this will ensure something still displays even if they forget it.

Testing

This change was tested using:

  • WDIO
  • Jest
  • Visual testing (please attach a screenshot or recording)
  • Other (please describe below)
  • No tests are needed

Reviews

In addition to engineering reviews, this PR needs:

  • UX review
  • Accessibility review
  • Functional review

Additional Details

This PR resolves:

UXPLATFORM-XXXX


Thank you for contributing to Terra.
@cerner/terra

@artpark artpark requested a review from a team January 23, 2024 19:47
@artpark artpark self-assigned this Jan 23, 2024
@artpark artpark changed the base branch from main to terra-application-v1 January 23, 2024 19:47
@@ -122,12 +122,17 @@ const ApplicationBase = ({

const { localeOverride, themeOverride } = useTestOverrides(); // Allows us to test deployed applications in different locales.

let density = themeDensity || themeConfig?.density;
if (!density && themeName === 'redwood-theme') {
Copy link
Contributor

@sycombs sycombs Jan 23, 2024

Choose a reason for hiding this comment

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

Small suggestion: If we swap the conditions

Suggested change
if (!density && themeName === 'redwood-theme') {
if (themeName === 'redwood-theme' && !density) {

then it will only check density when the theme is redwood since it will short-circuit and only the first condition needs to be evaluated.

@github-actions github-actions bot temporarily deployed to preview-pr-366 January 23, 2024 22:08 Destroyed
@artpark artpark merged commit 2327a39 into terra-application-v1 Jan 24, 2024
7 checks passed
@artpark artpark deleted the default-density branch January 24, 2024 20:24
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants