Add blankslate classes to replace utilities #1919
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
🦋 Changeset detectedLatest commit: 4fe418c The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
This is part of the effort to remove all usages of utility classes to improve consistency and prevent specificity bugs.
hectahertz
force-pushed
the
hectahertz/remove-css-utilities-blankslate
branch
from
04f56ad
to
1b800ea
Compare
hectahertz
commented
Feb 1, 2022
| @@ -27,22 +27,26 @@ When it helps the message, include (relevant) icons in your blank slate. Add the | |||
| <div class="blankslate"> | |||
| <!-- <%= octicon "octoface", :height = 24, :class => "blankslate-icon" %> --> | |||
| <svg class="octicon octicon-octoface blankslate-icon" aria-hidden="true" xmlns="http://www.w3.org/2000/svg" viewBox="0 0 24 24" width="24" height="24"><path d="M7.75 11c-.69 0-1.25.56-1.25 1.25v1.5a1.25 1.25 0 102.5 0v-1.5C9 11.56 8.44 11 7.75 11zm1.27 4.5a.469.469 0 01.48-.5h5a.47.47 0 01.48.5c-.116 1.316-.759 2.5-2.98 2.5s-2.864-1.184-2.98-2.5zm7.23-4.5c-.69 0-1.25.56-1.25 1.25v1.5a1.25 1.25 0 102.5 0v-1.5c0-.69-.56-1.25-1.25-1.25z"></path><path fill-rule="evenodd" d="M21.255 3.82a1.725 1.725 0 00-2.141-1.195c-.557.16-1.406.44-2.264.866-.78.386-1.647.93-2.293 1.677A18.442 18.442 0 0012 5c-.93 0-1.784.059-2.569.17-.645-.74-1.505-1.28-2.28-1.664a13.876 13.876 0 00-2.265-.866 1.725 1.725 0 00-2.141 1.196 23.645 23.645 0 00-.69 3.292c-.125.97-.191 2.07-.066 3.112C1.254 11.882 1 13.734 1 15.527 1 19.915 3.13 23 12 23c8.87 0 11-3.053 11-7.473 0-1.794-.255-3.647-.99-5.29.127-1.046.06-2.15-.066-3.125a23.652 23.652 0 00-.689-3.292zM20.5 14c.5 3.5-1.5 6.5-8.5 6.5s-9-3-8.5-6.5c.583-4 3-6 8.5-6s7.928 2 8.5 6z"></path></svg> | |||
| <h3>This is a blank slate</h3> | |||
There was a problem hiding this comment.
This one was missing the heading margin
Merged
1 task
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What are you trying to accomplish?
This is part of the effort to remove all usages of utility classes to improve consistency and prevent specificity bugs. There will be a follow up PR on
primer/view_componentsto use these onBlankslate. More context on https://github.com/github/primer/issues/585.There's a couple minor spacing issues that have been addressed too. They had been pointed out on (the component audit)[https://github.com/github/primer/issues/373#issuecomment-943927091].
Before
After
buttondon't collapse. As we usedivwrappers for actions now, this works and we have standard spacing.pas a way to getdisplay: blockand a bottom margin. As we usedivwrappers for actions now, this is also simplified and we can use a standard margin.What approach did you choose and why?
I decided to go for a minimally invasive set of changes so we can make these changes in all components with the least amount of friction possible. No breaking changes.
I found a couple inconsistencies that I'd like to mention.
Heading font size
As per the discussions on this issue it seems that we don't want to default to any size and keep the default heading styling. cc @pouretrebelle @ashygee
The
Blankslateview component adds ah2utility class by default but, because we want to remove this behavior, I won't add it toblankslate-headingand will remove it on theprimer/view_componentsPR.Default heading tag
In the
primer/cssdocs we default toh3everywhere, but this doesn't matchprimer/view_componentswhere we useh2in 99% of the cases. We might want to update the examples to keep things more consistent.What should reviewers focus on?
Please focus on the explanations on the previous point.
Are additional changes needed?