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

Modal manager redwood styling #1994

Merged
merged 8 commits into from
Jan 19, 2024
Merged

Modal manager redwood styling #1994

merged 8 commits into from
Jan 19, 2024

Conversation

adoroshk
Copy link
Contributor

@adoroshk adoroshk commented Jan 18, 2024

Summary

This PR adds redwood styling to the modal manager and adds wdio screenshots for that component.

NOTE: the redwood css styles that changed are border radius, background color and box-shadow css props of the disclosure modal, the content of the disclosure component is not impacted by the change. The wdio screenshots will need to be updated once the content components become redwood-styled.

Testing

This change was tested using:

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

Steps to test:

  1. Run npm ci to build and generate css styles for all themes.
  2. Switch to redwood theme.
  3. In developer tool observe that redwood values were applied to background-color, box-shadow, border-radius, and color props to the modal manager <div>.
Screenshot 2024-01-18 at 3 42 30 PM

Reviews

In addition to engineering reviews, this PR needs:

  • UX review
  • Accessibility review
  • Functional review

This PR resolves:
UXPLATFORM-10006

package.json Outdated
@@ -45,7 +45,7 @@
},
"overrides": [
{
"files": ["packages/terra-theme-properties/**/*.scss"],
"files": ["packages/terra-theme-properties/**/*.scss", "packages/**/redwood-theme/*.scss"],
Copy link
Contributor

@cm9361 cm9361 Jan 18, 2024

Choose a reason for hiding this comment

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

I think you want "...redwood-theme/**/*.scss"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch, fixed in dcf6be0

@@ -323,13 +323,14 @@

// =======================================
// terra-modal-manager
// https://github.com/cerner/terra-core/tree/main/packages/terra-modal-manager
// https://github.com/cerner/terra-framework/tree/main/packages/terra-modal-manager
Copy link
Contributor

Choose a reason for hiding this comment

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

These files are generated. I communicated this issue with the UX team. You don't want to manually update it as it would just be incorrect the next release.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Awesome, thank you so much for taking care of that, reverted in dcf6be0

// https://jira2.cerner.com/browse/UXPLATFORM-10006
// =======================================
.redwood-theme {
--modal-background-color: var(--container-style-1-background-color);
--modal-box-shadow: var(--rds-box-shadow-md);
--modal-border-radius: var(--rds-border-radius-md);
// TODO add --modal-foreground-color variable or remove this line is inherit color works
Copy link
Contributor

Choose a reason for hiding this comment

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

I still think we don't want a UX theme variable here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed here: dcf6be0

Copy link
Contributor

@sycombs sycombs left a comment

Choose a reason for hiding this comment

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

Just have one comment about the changelog, otherwise LGTM

@github-actions github-actions bot temporarily deployed to preview-pr-1994 January 19, 2024 19:00 Destroyed
@adoroshk adoroshk merged commit f8f0445 into main Jan 19, 2024
22 checks passed
@adoroshk adoroshk deleted the redwood-style-modal-manager branch January 19, 2024 19:41
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