search

aurelia / documentation

The documentation for Aurelia.
MIT License
105 stars 111 forks source link

Update dispatchify example to be stand-alone. #434

Closed baerrach closed 5 years ago

EisenbergEffect commented 5 years ago

@zewa666 Would you mind taking a quick look to make sure everything looks good here?

zewa666 commented 5 years ago

@baerrach thx for the PR, while I get the intention of providing full samples, I feel like that might distract from what the actual section wants to highlight. Did you have issues following along the original docs for the given section? Another alternative to full blown Code parts might be to add Codesandbox samples instead what do you think?

baerrach commented 5 years ago

@zewa666 I have really appreciated that the Aurelia documentation gives me examples that enshrine their best practices.

With this example, when I went to try the snippets there are too many short cuts being taken that make it non-obvious what the best practices should be.

Explicitly including imports is useful for a newbie like me who goes to cut-and-paste and doesn't know where to find the imports yet.

The actions are defined within the ViewModel file which seems to go against where you should place actions, and in fact doesn't work as written because they must be registered first in the Store, which then means app.js needs to be included in the example.

So including two extra files for the actions and store registration make the example minimally complete. And highlights the errors/misunderstandings with the example as it is currently.

Would this be better written as a code sanbox snippet? I'm not sure. I think this is small enough to live on its own. But I'm not fussed either way.

zewa666 commented 5 years ago

Yeah I see where you're going. Again the intention was to focus the reader on the topic at hand vs fully functional snippets. But you're most likely right we should make sure that copy&paste readers don't get stuck.

zewa666 commented 5 years ago

Ok I just checked out the sample and yeah I think it's not too overwhelming so if you could just fix those two comments above I think we're fine to merge this

baerrach commented 5 years ago

I'm not familiar enough yet where the name in .registerAction gets used.

I've found namespacing to be invaluable when looking through Redux devtools and console logs.

But having an opinionated opinion on that would be a new topic for discussion.

zewa666 commented 5 years ago

I do fully agree that namespacing definitely is helpful, I'm just hesitant to sneak in such a concept with a small sample. The use of the name is essentially the one to be displayed in the logs and DevTools.

@EisenbergEffect LGTM lets get this in ;)