-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
chore(Sidebar): use EventStack component #2990
Conversation
Signed-off-by: Oleksandr Fediashov <[email protected]>
Signed-off-by: Oleksandr Fediashov <[email protected]>
Signed-off-by: Oleksandr Fediashov <[email protected]>
Signed-off-by: Oleksandr Fediashov <[email protected]>
Signed-off-by: Oleksandr Fediashov <[email protected]>
Signed-off-by: Oleksandr Fediashov <[email protected]>
Signed-off-by: Oleksandr Fediashov <[email protected]>
Signed-off-by: Oleksandr Fediashov <[email protected]>
Signed-off-by: Oleksandr Fediashov <[email protected]>
Signed-off-by: Oleksandr Fediashov <[email protected]>
Codecov Report
@@ Coverage Diff @@
## master #2990 +/- ##
==========================================
- Coverage 99.92% 99.92% -0.01%
==========================================
Files 169 169
Lines 2801 2790 -11
==========================================
- Hits 2799 2788 -11
Misses 2 2
Continue to review full report at Codecov.
|
src/modules/Sidebar/Sidebar.js
Outdated
@@ -191,6 +162,7 @@ class Sidebar extends Component { | |||
<Ref innerRef={this.handleRef}> | |||
<ElementType {...rest} className={classes}> | |||
{childrenUtils.isNil(children) ? content : children} | |||
{visible && <EventStack name={'click'} on={this.handleDocumentClick} />} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
30 lines vs 1 line, exciting result ✌️
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very cool, I'll give this a proper review before merging. It looks like a fantastic idea :D
…React into feat/eventstack # Conflicts: # package.json # yarn.lock
I assume the future of this component will change now that we're publishing EventStack from elsewhere (eventually Stardust)? |
Yep, I will move this component to a package. |
…Org/Semantic-UI-React into feat/eventstack # Conflicts: # src/modules/Sidebar/Sidebar.js
…React into feat/eventstack
Signed-off-by: Oleksandr Fediashov <[email protected]>
Released in |
We have a cool API called EventStack. It's time to make it more awesome 🐱 This PR offers a new component exposes the EventStack API as public and provides a declarative way to manage it.
This will allow to reduce complexility with subscribtions to events and will allow our users to work with our API deeply.