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

feat: ComponentHeader component, component overview fixes, more #20

Merged
merged 30 commits into from Aug 14, 2019

Conversation

@TheSisb
Copy link
Collaborator

TheSisb commented Aug 1, 2019

Structural changes

  • Swap the default theme border-color light/dark token to match Starch
  • Shuffle around the folder and files in the paste-website package. Having folders of just index files is more cumbersome than just putting the file in that parent folder directly.
  • Removed website building from yarn build command, but added it to CircleCI. When we run yarn build locally, we usually don't want to build the website, so this makes it faster.
  • Our pre-push git hook now doesn't run build anymore. It took way too long.
  • Added yarn type-check to CircleCI and on pre-push git hook.
  • Fix some type-check errors.

Feature changes

  • Add breadcrumb component for doc site
  • Fixed border issues on Sidebar
  • Keep sidebar category open when navigating to subpages
  • Add ComponentHeader shortcode component
  • Improve ComponentOverviewTable design
@now

This comment has been minimized.

Copy link

now bot commented Aug 1, 2019

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

Latest deployment for this branch: https://paste-git-component-page-header.twilio-dsys.now.sh

@TheSisb TheSisb force-pushed the component-page-header branch from 0b28057 to 207e45e Aug 7, 2019
@now now bot had a problem deploying to staging Aug 7, 2019 Failure
@TheSisb TheSisb force-pushed the component-page-header branch from 207e45e to 6417ab7 Aug 9, 2019
@now now bot had a problem deploying to staging Aug 9, 2019 Failure
@now now bot had a problem deploying to staging Aug 9, 2019 Failure
@TheSisb TheSisb changed the title [wip] Component page header feat: ComponentHeader component, component overview fixes, more Aug 9, 2019
@now now bot had a problem deploying to staging Aug 9, 2019 Failure
@now now bot had a problem deploying to staging Aug 9, 2019 Failure
@TheSisb TheSisb requested review from SiTaggart and richbachman Aug 9, 2019
@now now bot had a problem deploying to staging Aug 9, 2019 Failure
@now now bot had a problem deploying to staging Aug 9, 2019 Failure
@TheSisb TheSisb force-pushed the component-page-header branch from 47c83c1 to ce84255 Aug 9, 2019
@now now bot had a problem deploying to staging Aug 13, 2019 Failure
Copy link
Collaborator

SiTaggart left a comment

I think in the future it would be super helpful if we broke something of this size down into different PRs. It's a little hard to parse as it does so many different things right now.

@TheSisb

This comment has been minimized.

Copy link
Collaborator Author

TheSisb commented Aug 14, 2019

Yeah, I totally agree. I hadn't realized at the time that it would balloon this much. I could still split it up if you'd like.

const BreadcrumbItem = styled(Link)`
height: 24px;
font-size: 14px;
line-height: 1.71;

This comment has been minimized.

Copy link
@SiTaggart

SiTaggart Aug 14, 2019

Collaborator

Can you write a TODO here to convert these to tokens? line-heights are coming and font-size should already cover it.

Also is the height going to cause a problem if you bump the base font size for the page? Will the font scale (when using rems from tokens) and the fixed height cause issues?

Copy link
Collaborator

SiTaggart left a comment

Apart from that one TODO, all good

@TheSisb TheSisb merged commit 875e124 into master Aug 14, 2019
6 checks passed
6 checks passed
Semantic Pull Request ready to be squashed
Details
ci/circleci: build Your tests passed on CircleCI!
Details
ci/circleci: lint Your tests passed on CircleCI!
Details
ci/circleci: prettier Your tests passed on CircleCI!
Details
ci/circleci: test Your tests passed on CircleCI!
Details
now Deployment has completed
Details
@TheSisb TheSisb deleted the component-page-header branch Aug 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.