-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Primer 10.2.0 #407
Primer 10.2.0 #407
Conversation
Point style field to built css
automatically style first and last breadcrumb
Updates to stylelint package links/docs for new structure
Add Sass key to package.json
Increase spacing scale
- [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The breadcrumb changes aren't correct, need to get them fixed before merging in.
CHANGELOG.md
Outdated
# 10.2.0 (2017-12-11) | ||
|
||
#### :rocket: Enhancement | ||
* [#376](https://github.com/primer/primer/pull/376) Increase spacing scale. ([@gladwearefriends](https://github.com/gladwearefriends)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you change the title to "Extend spacing scale for marketing" to make it a bit clearer?
@@ -12,6 +11,7 @@ | |||
} | |||
} | |||
|
|||
:last-child .breadcrumb-item, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change removes the forward slash off of every breadcrumb, unsure what this change was trying to achieve but we don't want to merge it like this.
@@ -1,6 +1,5 @@ | |||
.breadcrumb-item { | |||
display: inline-block; | |||
margin-left: -$spacer-1; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't want the margin left on the first breadcrumb, but we need it on the others, this change makes the spacing either side of the /
uneven.
@@ -1,11 +1,12 @@ | |||
{ | |||
"version": "6.0.2", | |||
"version": "6.0.3", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be a minor bump seeing as there are minor changes to packages within it?
@@ -1,6 +1,6 @@ | |||
.breadcrumb-item { | |||
display: inline-block; | |||
margin-left: -$spacer-1; | |||
margin-left: -0.35em; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🙈
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🙊.375
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
WIP pr for Primer 10.2.0 minor release
Tracking issue here: #406
Ship checklist