-
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
Importing more styles from Primer View Components #2326
Conversation
|
@@ -1 +1,2 @@ | |||
@import '@primer/view-components/app/components/primer/beta/blankslate'; | |||
@import '../support/index.scss'; | |||
@import './blankslate.scss'; |
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.
I wanted to put this import back after #2318 in case anyone is importing the src/blankslate/blankslate.scss
file directly so it's not breaking.
font-weight: $font-weight-bold; | ||
background-color: var(--color-attention-subtle); | ||
} | ||
@import '@primer/view-components/app/components/primer/beta/flash'; |
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.
Figured out what was wrong with those failures, removing the .css
here fixed it. We'll have to remember that it could cause problems
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.
One thing I was wondering: Should this be a major
change?
It could be just a patch
since the outputted CSS is still more or less the same. On the other hand, if you fork primer/css
and wanted to make changes to the .scss files, the source for lots of components doesn't exist anymore, so it might also warrant a major
version bump:
- Up to
20.7.1
-> you can still edit the source in .scss files - From
21.0.0
-> lots of components only exist as "read only"
This was meant to be a step that would prevent us from doing a major bump. I think calling it a patch or minor is good enough to let folks know the styles aren't changing. The only thing someone might loose is access to changing the sass variables. |
What are you trying to accomplish?
labels
styles to PVC view_components#1587Truncate
styles to PVC view_components#1590Breadcrumbs
styles to PVC view_components#1592ToggleSwitch
styles to PVC view_components#1593Subhead
styles to PVC view_components#1594TimelineItem
styles to PVC view_components#1598Popover
styles to PVC view_components#1599dropdown
styles to PVC view_components#1600flash
styles to PVC view_components#1601Avatar
+AvatarStack
styles to PVC view_components#1607autocomplete
styles to PVC view_components#1609Box
styles to PVC view_components#1618navigation
styles to PVC view_components#1626Bundle size changes
Selector diffs
What approach did you choose and why?
What should reviewers focus on?
Can these changes ship as is?