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

dev-9095 SCSS Refactor #1495

Open
wants to merge 2 commits into
base: dev
Choose a base branch
from
Open

dev-9095 SCSS Refactor #1495

wants to merge 2 commits into from

Conversation

joshlacey
Copy link
Collaborator

Testing Steps

Testing on this feature will mostly need to happen through regression. I've run BackstopJS against Storybook and compared Test branch against these changes. Styling wise there were some differences in spacing mostly.

There is a big change of doing a direct import of the Components into Editor and Dashboard. I did this so that we can avoid importing duplicate SCSS. This resulted in about a 50% reduction in the build size of the dashboard and editor packages when you run lerna run build. Also changes to both SCSS and Components now happens without needing to rebuild packages.

Self Review

  • I have added testing steps for reviewers
  • I have commented my code, particularly in hard-to-understand areas
  • My changes generate no new warnings
  • New and existing unit tests are passing

Screenshots (if applicable)

Additional Notes

Copy link
Collaborator

@adamdoe adamdoe left a comment

Choose a reason for hiding this comment

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

I think this looks good, the only style difference that stood out to me were tooltips on the step one screen. The background is black instead of white now. Nice care with organization and updating of variables. 💯

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.

2 participants