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

fix(docs): Issue 1120 docs #1231

Merged
merged 11 commits into from
Aug 29, 2022
Merged

fix(docs): Issue 1120 docs #1231

merged 11 commits into from
Aug 29, 2022

Conversation

Exalted100
Copy link
Contributor

@Exalted100 Exalted100 commented Aug 26, 2022

Related Issues

Fixes #.

Summary

Reviewed the following for grammatical errors and minor style changes:

  • testing.mdx
  • auto-integrating-selectors.md
  • event-handler-in-pre-react-18.md
  • flux-inspired-practice.md
  • immutable-state-and-merging.md

In those five documents, I encountered two broken links. Both were in flux-inspired-practice.md. I was able to replace one with the proper code link. However, I couldn't for the second, as I couldn't find the proper link. You can see it right before the last code block in the file, with the text [this example](https://codesandbox.io/s/amazing-kepler-swxol)

Check List

  • yarn run prettier for formatting code and docs

@codesandbox-ci
Copy link

codesandbox-ci bot commented Aug 26, 2022

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 1d58fbe:

Sandbox Source
React Configuration
React Typescript Configuration
React Browserify Configuration
React Snowpack Configuration
React Parcel Configuration

Copy link
Collaborator

@chrisk-7777 chrisk-7777 left a comment

Choose a reason for hiding this comment

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

Awesome stuff @Exalted100
My comments were only "non-actions", so from my perspective this can be merged

@@ -19,11 +19,11 @@ const useBoundStore = create((set) => ({
}))
```

See [Splitting the store into separate slices](https://github.com/pmndrs/zustand/blob/main/docs/typescript.md#slices-pattern) for how to define a store with separate slices.
See [Splitting the store into separate slices](https://github.com/pmndrs/zustand/blob/main/docs/guides/typescript.md#slices-pattern) for how to define a store with separate slices.
Copy link
Collaborator

Choose a reason for hiding this comment

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

[no action] This highlights a "post launch" action. Once the docs are all squared away, we may need to do a pass and update the links to point to the docs themselves rather than back to the repo.

Copy link
Member

@dai-shi dai-shi Aug 28, 2022

Choose a reason for hiding this comment

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

@chrisk-7777 We want to make it work to view docs in github. I believe relative paths should work.

Suggested change
See [Splitting the store into separate slices](https://github.com/pmndrs/zustand/blob/main/docs/guides/typescript.md#slices-pattern) for how to define a store with separate slices.
See [Splitting the store into separate slices](./typescript.md#slices-pattern) for how to define a store with separate slices.

@@ -46,12 +46,12 @@ const dispatch = useGrumpyStore((state) => state.dispatch)
dispatch({ type: types.increase, by: 2 })
```

Or, just use our redux-middleware. It wires up your main-reducer, sets initial state, and adds a dispatch function to the state itself and the vanilla api. Try [this](https://codesandbox.io/s/amazing-kepler-swxol) example.
You could also use our redux-middleware. It wires up your main reducer, sets initial state, and adds a dispatch function to the state itself and the vanilla api. Check [this example](https://codesandbox.io/s/amazing-kepler-swxol).
Copy link
Collaborator

Choose a reason for hiding this comment

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

[no action] Small thing, but these longer links are great for accessibility I believe ("this example" vs previous "this") 👍

Copy link
Member

@dai-shi dai-shi left a comment

Choose a reason for hiding this comment

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

Thanks for your work!

@@ -19,11 +19,11 @@ const useBoundStore = create((set) => ({
}))
```

See [Splitting the store into separate slices](https://github.com/pmndrs/zustand/blob/main/docs/typescript.md#slices-pattern) for how to define a store with separate slices.
See [Splitting the store into separate slices](https://github.com/pmndrs/zustand/blob/main/docs/guides/typescript.md#slices-pattern) for how to define a store with separate slices.
Copy link
Member

@dai-shi dai-shi Aug 28, 2022

Choose a reason for hiding this comment

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

@chrisk-7777 We want to make it work to view docs in github. I believe relative paths should work.

Suggested change
See [Splitting the store into separate slices](https://github.com/pmndrs/zustand/blob/main/docs/guides/typescript.md#slices-pattern) for how to define a store with separate slices.
See [Splitting the store into separate slices](./typescript.md#slices-pattern) for how to define a store with separate slices.

@dai-shi
Copy link
Member

dai-shi commented Aug 28, 2022

https://codesandbox.io/s/amazing-kepler-swxol

Probably the author of the sandbox removed it. @drcmda is it you?

This is also in readme.md. If we don't know how to recover it, let's remove links.

@dai-shi dai-shi merged commit daf52ed into pmndrs:main Aug 29, 2022
@dai-shi dai-shi changed the title Issue 1120 docs fix(docs): Issue 1120 docs Aug 29, 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.

3 participants