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

Adding the Header component to primer #845

Merged
merged 11 commits into from
Aug 8, 2019
Merged

Conversation

jonrohan
Copy link
Member

@jonrohan jonrohan commented Jul 22, 2019

This ports the recently created Header component from .com and adds documentation on it.

TODO:

  • Include octicon css #857 Include the @primer/octicons css in the build process. There's style defaults `fill: currentColor;display: inline-block;vertical-align: text-bottom" that will fix a bug in the header example in the docs.

/cc @primer/ds-core

@vercel
Copy link

vercel bot commented Jul 22, 2019

This pull request is automatically deployed with Now.
To access deployments, click Details below or on the icon next to each push.

@jonrohan jonrohan requested a review from shawnbot July 22, 2019 20:52
@jonrohan jonrohan added the css label Jul 30, 2019
@shawnbot shawnbot mentioned this pull request Jul 30, 2019
11 tasks
@jonrohan jonrohan added this to the v3.0.0 milestone Jul 30, 2019
@primer primer locked and limited conversation to collaborators Jul 31, 2019
@primer primer unlocked this conversation Jul 31, 2019
shawnbot added a commit that referenced this pull request Aug 7, 2019
@shawnbot shawnbot mentioned this pull request Aug 7, 2019
@shawnbot shawnbot changed the base branch from master to release-12.6.0 August 7, 2019 20:50
shawnbot added a commit that referenced this pull request Aug 7, 2019
shawnbot added a commit that referenced this pull request Aug 7, 2019
src/header/README.md Outdated Show resolved Hide resolved
src/header/README.md Outdated Show resolved Hide resolved
shawnbot added a commit that referenced this pull request Aug 7, 2019
Co-Authored-By: Shawn Allen <shawn.allen@github.com>
src/header/README.md Outdated Show resolved Hide resolved
Co-Authored-By: Shawn Allen <shawn.allen@github.com>
@vercel vercel bot temporarily deployed to staging August 8, 2019 18:45 Inactive
Copy link
Contributor

@shawnbot shawnbot left a comment

Choose a reason for hiding this comment

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

The canonical example includes some unnecessary v-align-middle funkiness that I think we could eliminate with flexbox. I hacked this in the live example and it seems to work well:

image

Suggested changes apply the fix, but you'll probably want to test it more thoroughly.

pages/css/components/header.md Outdated Show resolved Hide resolved
pages/css/components/header.md Outdated Show resolved Hide resolved
src/header/header.scss Show resolved Hide resolved
@jonrohan
Copy link
Member Author

jonrohan commented Aug 8, 2019

The canonical example includes some unnecessary v-align-middle funkiness that I think we could eliminate with flexbox. I hacked this in the live example and it seems to work well:

I'll update to use the flexbox utilities, but want to steer clear of adding to the link class, worried it will break a lot of things.

@vercel vercel bot temporarily deployed to staging August 8, 2019 19:39 Inactive
@jonrohan jonrohan requested a review from shawnbot August 8, 2019 19:40
Copy link
Contributor

@shawnbot shawnbot left a comment

Choose a reason for hiding this comment

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

This is great, let's ship it! 🚀

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