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

#215 add ToastHub component #249

Merged
merged 23 commits into from Nov 13, 2018

Conversation

Projects
None yet
4 participants
@drcmda
Copy link
Contributor

drcmda commented Nov 9, 2018

Old history: #243 (243 was orphaned unfortunately)

Reference: #215

Adds a MessageHub component to AragonUI, documentation and running example included.

messagehub

drcmda added some commits Nov 6, 2018

@drcmda drcmda changed the title #215 add MessageHub component #215 add ToastHub component Nov 9, 2018

@drcmda

This comment has been minimized.

Copy link
Contributor

drcmda commented Nov 9, 2018

@sohkai i've named the spring config "animationConfig", as for the other props, showIndicator, timeout and so on, should they remain single props or join a config object?

@bpierre

This comment has been minimized.

Copy link
Member

bpierre commented Nov 9, 2018

@drcmda @sohkai What about transitionConfig or springConfig instead?

Also, exposing the spring config is something that we don’t do for the other components. We could decide to start doing it, but I’m not sure we should let people change the spring configurations of “high level” components (as opposed to “low level” components like Popover), even after the move to Lorikeet and a proper theming system. The general feel depends a lot on the transitions, and I think it is important to have a consistent feel across different apps using the toolkit (e.g. Material Design which is quite strict about this).

About the other props, there is no strict rule yet, but we usually keep them as single props.

@drcmda

This comment has been minimized.

Copy link
Contributor

drcmda commented Nov 9, 2018

Oh, in that case i think it would be fine to remove the prop and default to springs.lazy. Would be strange if this one does it different than the bunch.

@bpierre bpierre requested review from sohkai and bpierre Nov 12, 2018

sohkai added some commits Nov 12, 2018

@sohkai

This comment has been minimized.

Copy link
Member

sohkai commented Nov 12, 2018

@drcmda I've added two commits to remove animationConfig from the README, and to fix the linter :).

drcmda added some commits Nov 13, 2018

@sohkai

This comment has been minimized.

Copy link
Member

sohkai commented Nov 13, 2018

Ahh @drcmda let's use stylingProps, since that'll also get rid of the linter warnings for the styling props.

Sorry for the churn 🙇!

@bpierre bpierre merged commit ec45bb4 into aragon:master Nov 13, 2018

1 of 2 checks passed

continuous-integration/travis-ci/pr The Travis CI build could not complete due to an error
Details
license/cla Contributor License Agreement is signed.
Details
@bpierre

This comment has been minimized.

Copy link
Member

bpierre commented Nov 13, 2018

Merged! 💥

Thank you @drcmda, I can’t wait to start using it! 🎉

@gitcoinbot gitcoinbot referenced this pull request Nov 20, 2018

Closed

Toast component #215

1 of 3 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment