Skip to content
This repository has been archived by the owner on Jan 23, 2024. It is now read-only.

docs(color-mode): updates docsite to reflect current behavior #1062

Merged
merged 9 commits into from
Dec 30, 2022

Conversation

tresorama
Copy link
Contributor

@tresorama tresorama commented Oct 7, 2022

📝 Description

Update doc page for Color Mode to reflect current behavior.

⛳️ Current behavior (updates)

Current doc suggest a wrong use of the Color Mode API.
Current docs refers to Chakra v1 behavior of the Color Mode, not to v2.

🚀 New behavior

Help newcomers to understand current behavior and avoid confusion.

Proposed changes:

  • Suggest common usages of the theme config ready to copy/paste, with the idea to remove confusion.
  • Add an introduction to Color Mode
  • Clarify default value of theme config options.
  • Link a playground for manual testing of theme config options.

💣 Is this a breaking change (Yes/No): No

📝 Additional Information

Chakra v2 updated the code (as stated here and here ) and confirmed by a codesandbox i created to test).

There are two sentences that need to be reviewed, these will be commented in PR comments.

tresorama

@vercel
Copy link

vercel bot commented Oct 7, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
chakra-ui-docs ✅ Ready (Inspect) Visit Preview Dec 29, 2022 at 11:31PM (UTC)

@tresorama tresorama marked this pull request as ready for review October 13, 2022 12:59
content/docs/styled-system/color-mode.mdx Outdated Show resolved Hide resolved
content/docs/styled-system/color-mode.mdx Outdated Show resolved Hide resolved
content/docs/styled-system/color-mode.mdx Outdated Show resolved Hide resolved
content/docs/styled-system/color-mode.mdx Outdated Show resolved Hide resolved
content/docs/styled-system/color-mode.mdx Outdated Show resolved Hide resolved
content/docs/styled-system/color-mode.mdx Outdated Show resolved Hide resolved
content/docs/styled-system/color-mode.mdx Outdated Show resolved Hide resolved
content/docs/styled-system/color-mode.mdx Outdated Show resolved Hide resolved
content/docs/styled-system/color-mode.mdx Outdated Show resolved Hide resolved
content/docs/styled-system/color-mode.mdx Outdated Show resolved Hide resolved
@tresorama
Copy link
Contributor Author

Adding this comment to make clear that requested changes are now included in 1d1ec8b commit , all but one.

The one still unresolved is #1062 (review)

@tresorama tresorama requested a review from aacevski November 19, 2022 17:13
@aacevski
Copy link
Contributor

Other than the comment I wrote, everything looks okay on my end! When it has been resolved we can merge this!

@tresorama
Copy link
Contributor Author

tresorama commented Dec 26, 2022

Other than the comment I wrote, everything looks okay on my end! When it has been resolved we can merge this!

Great to hear that !
I'm excited to contribute to this great library.

My feature branch is behind main. How should i approach this?
Rebase chakra-ui:main into tresorama:docs/color-mode ?
Doing nothing ?
Other ?

@tresorama tresorama requested a review from aacevski December 26, 2022 17:27
@aacevski
Copy link
Contributor

aacevski commented Dec 27, 2022

Other than the comment I wrote, everything looks okay on my end! When it has been resolved we can merge this!

Great to hear that ! I'm excited to contribute to this great library.

My feature branch is behind main. How should i approach this? Rebase chakra-ui:main into tresorama:docs/color-mode ? Doing nothing ? Other ?

Hey, you need to go to your fork and click this button and then merge the main branch into yours.
image

@tresorama
Copy link
Contributor Author

Merged. Let me know if something else need to be done on my end.

Copy link
Contributor

@aacevski aacevski left a comment

Choose a reason for hiding this comment

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

image

One small thing, these two look too much in my opinion, can you maybe make the first Alert as a text and see how it looks?

Copy link
Contributor

@aacevski aacevski 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 perfect, thanks a lot. Merging this 🚀

@aacevski aacevski merged commit 8bebe25 into chakra-ui:main Dec 30, 2022
@tresorama
Copy link
Contributor Author

This is perfect, thanks a lot. Merging this 🚀

Awesome !
Glad to give something back to Chakra 🤙

Happy end of year Andrej.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants