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

v3 - Chakra-UI v1 & Next.js v10 #210

Closed
wants to merge 28 commits into from
Closed

v3 - Chakra-UI v1 & Next.js v10 #210

wants to merge 28 commits into from

Conversation

franky47
Copy link
Member

@franky47 franky47 commented Nov 13, 2020

  • Port existing components to Chakra-UI v1 API
  • Simplify _app.ts
  • Introduce _document.ts to add colorMode persistance
  • Add shorthands for Card layout types: <Card.Flex>, <Card.Stack> etc.. -> No need, replaced by the as prop
  • Add Image component (see Chakra-UI v1 🤝 Next.js v10 #187)
  • Use forwardRef for custom components
  • Mark support for as in Links as optional
  • Handle route links in menus (Menu Links #221)

Documentation

  • Migration guide
  • OutgoingLink rel options

Closes #187, #221.

@franky47 franky47 marked this pull request as draft November 13, 2020 17:32
Starting Chakra v1 migration
BREAKING CHANGE: `SvgBox` renamed `Svg`
BREAKING CHANGE: Replaced `PseudoBox` by `Box` (Chakra-UI v1)
BREAKING CHANGE: Removed `Container`, `StackContainer` and `FlexContainer`.
BREAKING CHANGE: `enableColorMode` is now part of Chakra (in `theme.config`)
BREAKING CHANGE: `globalCss` is now part of Chakra (in `theme.styles.global`)
BREAKING CHANGE: `getGlobalConfig` is deprecated
and spread around in Chakra's config & global styles.
Somehow the new Chakra UI styles don't play well with
JSDOM's CSS resolution, and textDecoration is not read
correctly. Runtime tests show that the correct styles are
applied, so we test the logic against another property
(color, which seems to go through).
@coveralls
Copy link

coveralls commented Nov 15, 2020

Pull Request Test Coverage Report for Build 364569309

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 34 of 37 (91.89%) changed or added relevant lines in 8 files are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage increased (+0.7%) to 91.824%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/components/image.tsx 4 7 57.14%
Files with Coverage Reduction New Missed Lines %
src/pages/_app.tsx 1 50.0%
Totals Coverage Status
Change from base Build 361788495: 0.7%
Covered Lines: 107
Relevant Lines: 113

💛 - Coveralls

@@ -72,6 +72,9 @@
"verbose": true,
"preset": "ts-jest/presets/js-with-ts",
"testEnvironment": "jsdom",
"setupFiles": [
"./test/env.js"
Copy link
Member Author

Choose a reason for hiding this comment

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

Note: This is a workaround for Next.js image config not having a default, should be resolved in the next version:
vercel/next.js#18373 (comment)

@trevorblades
Copy link

Stoked for this release! Is there any way you could publish a prerelease version off of this branch? It seems like the repo is set up to auto deploy, but I thought I'd ask anyway. If the answer is no, I'm happy to be patient 😇

@franky47
Copy link
Member Author

Hey Trevor, that sounds like a good idea, I'll look into how to do this. If you have experience with semantic-release, any help is welcome!

@franky47
Copy link
Member Author

franky47 commented Dec 2, 2020

@trevorblades you can give it a try: @47ng/[email protected], feel free to open an issue if you encounter any problems. Breaking changes are listed in the changelog.

Note to self: deploying pre-releases is extremely simple with semantic-release: enable CD to run on the beta branch, then point that branch to whatever you want to deploy, it will semver up and handle dist-tags automatically. 🚀

@trevorblades
Copy link

That's great to hear @franky47! I meant to investigate this, but you got to it before me. Thanks 😄

Doing so adds `next/image` in the imports tree,
which is enough to automatically opt-in to Next.js
Image Optimization feature, which has drawbacks:
- Breaks static export
- Requires a server / serverless setup
- Requires setting up the Image Optimization config

Buying in the Image component should be explicit,
so imports will be separated.

See #187.
@franky47
Copy link
Member Author

franky47 commented Dec 11, 2020

Hey @trevorblades, as a heads-up, I'm changing the way the Image component is included.

In 3.0.0-beta.1, it was included at the top-level with the rest, but this has a major drawback: it automatically opts-in to the Next.js Image Optimization feature, even if no Image component is actually used (simply importing '@47ng/chakra-next' made static exports impossible for example, as Image Optimization requires a server or an image loader).

So I'm moving it out of the top-level, the component can be accessed in 3.0.0-beta.2 the following way:

import { Image } from '@47ng/chakra-next/dist/components/image'

@franky47 franky47 closed this Feb 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Chakra-UI v1 🤝 Next.js v10
3 participants