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

Router store diagram #2287

Closed
wants to merge 1 commit into from
Closed

Conversation

Jefiozie
Copy link
Contributor

@Jefiozie Jefiozie commented Dec 4, 2019

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?
Adds a diagram for the flow of actions within the router store

[ ] Bugfix
[ ] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[x] Documentation content changes
[ ] Other... Please describe:

What is the current behavior?

Closes #

What is the new behavior?

Does this PR introduce a breaking change?

[ ] Yes
[x] No

Other information

@timdeschryver here is the PR for the diagram, I merged my branch with the changes for the action creators. This because I made changes to the docs there, I think this should be merged before this one is merged #2272

@ngrxbot
Copy link
Collaborator

ngrxbot commented Dec 4, 2019

Preview docs changes for 95c2b74 at https://previews.ngrx.io/pr2287-95c2b74/

@Jefiozie Jefiozie changed the title Router store schema Router store diagram Dec 4, 2019
Copy link
Member

@timdeschryver timdeschryver left a comment

Choose a reason for hiding this comment

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

Seems like you took some comits over from the other PR to this PR.
Could you only make the change towards the illustration, and revert the rest of the changes?

@Jefiozie
Copy link
Contributor Author

Jefiozie commented Dec 5, 2019

Sure thing, I did it on purposes as I also changed some stuff in the other PR that is a little bit related to this one.

@Jefiozie Jefiozie force-pushed the router-store-schema branch from 9f4aa8c to 25dfd96 Compare December 5, 2019 11:57
@Jefiozie Jefiozie force-pushed the router-store-schema branch from 25dfd96 to 95c2b74 Compare December 5, 2019 11:58
@Jefiozie
Copy link
Contributor Author

Jefiozie commented Dec 5, 2019

@timdeschryver done 😄

@timdeschryver
Copy link
Member

Pinging @brandonroberts @wesleygrimes @jordanpowell88 since ya'll were involved for the action flow diagram.

@wesleygrimes
Copy link
Contributor

I would love to see this match the diagram font and colors that @jordanpowell88 did. Jordan did you ever upload the PSD and/or AI files?

@Jefiozie
Copy link
Contributor Author

Jefiozie commented Dec 8, 2019

Sure, if there are some assets that I can use for that I can change it

@jordanpowell88
Copy link
Contributor

@wesleygrimes @Jefiozie @timdeschryver I would be happy to upload the psd file. Where would you like for me to put it?

@Jefiozie
Copy link
Contributor Author

Jefiozie commented Dec 9, 2019

If there is a need to have like a default kit for people to use, I would say let's make a "Press Kit" available via the ngrx.io site.

If you can share it via wetransfer of any other way I'm totally fine with it.

@jordanpowell88
Copy link
Contributor

Do we want to put the PSD file directly in a "Press Kit" folder in the repository? Or do we want to just link to an external download like a CDN or some other file sharing service? If that is the case is their something like this already set up? @timdeschryver

@Jefiozie
Copy link
Contributor Author

I've not yet found something like this for the ngrx site. Maybe we can start by sharing the psd so that we can create the diagram?(or maybe you can recreate the diagram and share the png).

I can create a issue to create a press kit page on the ngrx site? (And maybe even make it 😁)

@brandonroberts
Copy link
Member

A press kit page that includes the diagrams would be great.

@brandonroberts
Copy link
Member

Now that #2296 has landed, you can add the diagram to that page.

@brandonroberts
Copy link
Member

Reopen this when you're ready.

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.

6 participants