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.

Sign up for GitHub

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

Jump to bottom

fix: update default styles to be less different from edx.org brand #650

Merged
merged 5 commits into from Mar 4, 2021
Merged

fix: update default styles to be less different from edx.org brand #650

merged 5 commits into from Mar 4, 2021

Conversation

abutterworth
Copy link
Contributor

@abutterworth abutterworth commented Mar 2, 2021 •
edited

Give edx.org developers higher confidence that what they ship in open edx applications will look similar when the edx.org brand is applied by reducing major differences between paragon default styles and edx brand styles.:

  • Match font sizes in the type ramp (though line-heights are slightly different)
  • Update colors to utilize the the color system in a similar way to edx.org. Notably: adds a magenta “brand” color.

The differences we'll maintain:

  • Font family (system default san-serif vs. edx.org's use of Inter)
  • Border radii. (rounded buttons vs. edx.org's square)

image
image

@coveralls
Copy link

coveralls commented Mar 2, 2021 •
edited

Coverage Status

Coverage remained the same at 89.134% when pulling 8ec7f07 on abutterworth/default-colors-type into 1f38b51 on master.

@abutterworth abutterworth changed the title fix: update default colors to be less different from edx fix: update default styles to be less different from edx.org brand Mar 3, 2021
@abutterworth abutterworth requested a review from a team March 3, 2021 22:27
raq929
raq929 approved these changes Mar 3, 2021
View reviewed changes
Copy link
Contributor

@raq929 raq929 left a comment

Choose a reason for hiding this comment

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

Thaaanks, I think this will make things way easier.

scss/core/_variables.scss
$h4-mobile-font-size: $font-size-base * 1.375 !default;
$h5-mobile-font-size: $h5-font-size !default;
$h6-mobile-font-size: $h6-font-size !default;
$line-height-base: 1.55555555555555 !default;
Copy link
Contributor

Choose a reason for hiding this comment

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

Wait, what? Why?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Matttttttttthhhhhssssss. We would like our line-height base to net out at 28px like the design spec. As Bootstrap is today I think we're required to use a unit-less line-height because some math with another unit is done with it later. 1.5555 repeating * 18px is 28px.

@abutterworth abutterworth merged commit 9680e0b into master Mar 4, 2021
@abutterworth abutterworth deleted the abutterworth/default-colors-type branch March 4, 2021 13:51
@edx-semantic-release
Copy link
Contributor

🎉 This PR is included in version 13.15.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@raq929 raq929 raq929 approved these changes

Assignees
No one assigned
Labels
released
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@abutterworth @coveralls @edx-semantic-release @raq929