Add new onAlert prop

[mattcorner]

Description

Added onAlert prop which is (message, variant) => {}

Implements #200

Type of change

• New feature (non-breaking change which adds functionality)

How Has This Been Tested

• Docs updated with console.log callback on onAlert

Checklist

• My code follows the style guidelines of this project
• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• My changes generate no new warnings
• I have added tests that prove my fix is effective or that my feature works
• New and existing unit tests pass locally with my changes
• Any dependent changes have been merged and published in downstream modules

[mattcorner]

Found the entry to contributing a bit painful. Couple thoughts:

• Why no prettier and editorconfig? My local settings conflict massively with the eslint requirements of this project which made a simple change pretty painful.
• Pull request form talks about tests, I can't see any?
• How do you test locally? A sandbox / create-react-app to test the component in a sandbox during development would be useful i think?
• Whats the use of this gitmoji requirement? It doesn't show up in my git client so is a bit messy
• I get a whole bunch of dist/* files, package-lock.json files when developing this that I don't want to commit (i assume). Should these be in the .gitignore?

[panz3r]

Hi @mattcorner,

Thanks for your contribution and for your feedback, I'll try to answer your points below.

Why no prettier and editorconfig? My local settings conflict massively with the eslint requirements of this project which made a simple change pretty painful.

This is something I'd like to introduce as well, I was waiting to have a kind of "stable" code situation (with no open PRs) before doing a complete prettier refactor.
Also because a complete rewrite of the core components is in progress I'd like to wait a bit more, but we'll definitely setup linting (using both ESLint and Prettier).

Pull request form talks about tests, I can't see any?

This is another good point, the PR template was taken from a pre-made one but I'd also like to add some tests soon enough (no timeframe on this unfortunately).

How do you test locally? A sandbox / create-react-app to test the component in a sandbox during development would be useful i think?

We introduced React Styleguidist also because of this reason, so we have a test environment which doubles as documentation, just run yarn docs:dev and you can play around with the components easily, also consider that adding an example app would also mean maintaining more code (beside the module and docs one).

Whats the use of this gitmoji requirement? It doesn't show up in my git client so is a bit messy

Gitmoji is just one of many conventions that can be used to write commit messages but it's more "specific", there are more than 50 well defined "buckets" to classify a commit and - as per the convention details page here -

Using emojis on commit messages provides an easy way of identifying the purpose or intention of a commit with only looking at the emojis used.

On a side note about this, your second commit should be better classified as "Adding or updating types (Flow, TypeScript)" so the correct emoji would be 🏷️ instead of ✨ (used for a new feature).

I get a whole bunch of dist/* files, package-lock.json files when developing this that I don't want to commit (i assume). Should these be in the .gitignore?

Files under the dist folder are not to be committed but they cannot be added to the .gitignore otherwise the Rebuild Dist Github Action wouldn't be able to commit them after a successful build.
This files are required because there's no clear schedule for releases to npm (until PR #189 will be merged at least) and so to allow users of this module to use the latest features we need to commit the transpiled version so that they can install it directly from Github.

I hope this helps.

[panz3r]

LGTM 👍

[max-carroll]

Hmm, I would argue this is not basic usage, however until we have something like story book this is probably the best way to test it out. Perhaps when we have storybook we can have one example with all the customisation in

[mattcorner]

Agree, simply lack of a better place.