[Feature Notification] Add a euiAnimation method on the EuiHeaderSectionItemButton ref by elizabetdev · Pull Request #4424 · elastic/eui

elizabetdev · 2021-01-14T12:21:41Z

Summary

This PR adds a euiAnimation() method on the EuiHeaderSectionItemButton ref. This new method is required to accomplish the Notifications Template.

It also:

improves the docs
exposes the EuiHeaderSectionItemButton props table
adds a new snippet

Checklist

kibanamachine · 2021-01-14T12:42:53Z

Preview documentation changes for this PR: https://eui.elastic.co/pr_4424/

kibanamachine · 2021-01-14T14:11:36Z

Preview documentation changes for this PR: https://eui.elastic.co/pr_4424/

src/components/header/header_section/header_section_item_button.tsx

kibanamachine · 2021-01-15T19:48:13Z

Preview documentation changes for this PR: https://eui.elastic.co/pr_4424/

cchaos

Looks great! I like that the animation can be applied to any content within the button.

Design-wise, the one thing I noticed is that mobile version for the notification badge is not lining up with the dot version. Could you fix this? Maybe it means just using the JS breakpoint utility to show/hide on vs the other instead of custom CSS.

cchaos · 2021-01-15T19:58:41Z

src-docs/src/views/header/header_alert.js

+    setTimeout(() => {
+      setIsAnimating(true);
+    }, 100);


I think this is an important piece that consumers will not see. This is the only way they'd be able to reset the animation after each go. I think there's a couple paths:

Make sure to document this somewhere, maybe a prop comment will suffice

Is there any way to reset this within the component itself, maybe the prop is a callback function or something that passes back when the animation is done.

Thanks @cchaos,

I'm not completely sure if this is the best example to set true/false. I'm just trying to avoid a setTimeout of 5s. So the .euiHeaderSectionItemButton__content--isAnimating stays there forever, until the animation is triggered again. It doesn't seem right to have the class there when it's not animating.

Maybe there is.

@chandlerprall can you recommend what you believe is the best way to inject the CSS class .euiHeaderSectionItemButton__content--isAnimating and remove it after the animation is over (5s).

Instead of a prop which needs to toggle off/on to animate, we could add a method to the component's ref,

headerRef = useRef(); <HeaderUpdates ref={headerRef} showNotification={showNotification} setShowNotification={() => setShowNotification()} notificationsNumber={notificationsNumber} /> // trigger animation when notification count changes useEffect( () => headerRef.animateAlertIcon(), [notifications.length] )

If that sounds better I can put that together. Alternatively, we could use the animationend or transitionend events to trigger a callback to the parent, telling it to reset the animation prop back to false. This would avoid any setTimeout, but still requires extra boilerplate code in the consuming app

@cchaos what do you think of this approach?

I think that approach works. My only concern would be discoverability. Of course we'd have it hooked up in our docs, but would there be any autocompletion in IDE's that would help surface this option.

PR showing the method-based approach at elizabetdev#8

Thanks @chandlerprall. I found it a really bit difficult to document/exemplify this solution. So do you think this would work elizabetdev#9?

src-docs/src/views/header/header_example.js

src/components/header/header_section/header_section_item_button.tsx

kibanamachine · 2021-01-18T16:23:52Z

Preview documentation changes for this PR: https://eui.elastic.co/pr_4424/

kibanamachine · 2021-01-18T17:26:10Z

Preview documentation changes for this PR: https://eui.elastic.co/pr_4424/

kibanamachine · 2021-01-19T10:59:19Z

Preview documentation changes for this PR: https://eui.elastic.co/pr_4424/

…ethod refactor animation prop into a ref method

kibanamachine · 2021-02-01T11:47:46Z

Preview documentation changes for this PR: https://eui.elastic.co/pr_4424/

src-docs/src/views/header/header_alert.js

kibanamachine · 2021-02-01T13:20:58Z

Preview documentation changes for this PR: https://eui.elastic.co/pr_4424/

elizabetdev · 2021-02-01T13:49:13Z

src-docs/src/views/header/header_example.js

+            <EuiCode>true</EuiCode> to render a simple dot. You can also animate
+            the button by calling the <EuiCode>triggerAnimation()</EuiCode>{' '}
+            method on the <strong>EuiHeaderSectionItemButton</strong>{' '}
+            <EuiCode>ref</EuiCode>.


New explanation on how to animate. Is this enough?

kibanamachine · 2021-02-01T13:50:06Z

Preview documentation changes for this PR: https://eui.elastic.co/pr_4424/

… notifications

cchaos

Quick PR for you elizabetdev#11 that just splits off the notification and animation stuff into it's own example for better clarity.

Cleaned up some logic and created a docs example specifically for the notifications

elizabetdev · 2021-02-10T12:17:37Z

Thanks for this PR @cchaos, I merged it. I think @chandlerprall wants to change a few things but hopefully is not going to impact your work.

kibanamachine · 2021-02-10T12:34:24Z

Preview documentation changes for this PR: https://eui.elastic.co/pr_4424/

chandlerprall

Handful of small changes to the examples/snippets. I also threw a PR your way to re-enable the animation test. Everything else LGTM!

src-docs/src/views/header/header_animate.js

src-docs/src/views/header/header_example.js

src-docs/src/views/header/header_alert.js

src-docs/src/views/header/header_animate.js

src-docs/src/views/header/header_example.js

…test Re-enable the notification header button animation test

Co-authored-by: Chandler Prall <chandler.prall@gmail.com>

kibanamachine · 2021-02-10T19:36:28Z

Preview documentation changes for this PR: https://eui.elastic.co/pr_4424/

kibanamachine · 2021-02-10T20:10:12Z

Preview documentation changes for this PR: https://eui.elastic.co/pr_4424/

chandlerprall

Changes LGTM; pulled & tested locally

cchaos

Looks great!

src-docs/src/views/header/header_example.js

Co-authored-by: Caroline Horn <549577+cchaos@users.noreply.github.com>

kibanamachine · 2021-02-11T16:31:18Z

Preview documentation changes for this PR: https://eui.elastic.co/pr_4424/

kibanamachine · 2021-02-11T17:19:13Z

Preview documentation changes for this PR: https://eui.elastic.co/pr_4424/

* [Feature Notification] Add EuiNotificationEventMeta (#4433) * Add notification event meta * WIP * adding i18n * Fixing axe error in docs example * Adding context menus items * contetx menu item anchor position * Adding tests * Update src/components/notification/notification_event_meta.tsx Co-authored-by: Caroline Horn <549577+cchaos@users.noreply.github.com> * Improving code based on PR review * More PR review * Better description * Update src/components/notification/notification_event_meta.tsx Co-authored-by: Caroline Horn <549577+cchaos@users.noreply.github.com> * Update src-docs/src/views/notification_events/notification_events.js Co-authored-by: Caroline Horn <549577+cchaos@users.noreply.github.com> * Badge 100% width * ES lint * Read and unread states * Make sure overflow menu stays in the top right * Improving a11y * Adding playground for event read button * Removing EuiNotificationEventMeta from exported on index * Add a behavior unit test, update context menu test to snapshot the menu items * Update src/components/notification/notification_event_meta.tsx Co-authored-by: Chandler Prall <chandler.prall@gmail.com> * Update src/components/notification/notification_event_read_button.tsx Co-authored-by: Chandler Prall <chandler.prall@gmail.com> Co-authored-by: Caroline Horn <549577+cchaos@users.noreply.github.com> Co-authored-by: Chandler Prall <chandler.prall@gmail.com> * [Feature Notification] Adding animation prop to EuiHeaderSectionItemButton (#4424) * Adding animation and hasBackground props to EuiHeaderSectionItemButton * A few improvements * Docs props * Removing hasBackground prop * Using JS do hide and show on xs breakpoints * Adding a snippet * refactor animation prop into a ref method * Snapshots. Trigger animation comments. * Deleting unused css animation * triggerAnimation explanation * Cleaned up some logic and created a docs example specifically for the notifications * re-enable the animation test * Update src-docs/src/views/header/header_animate.js Co-authored-by: Chandler Prall <chandler.prall@gmail.com> * Update src-docs/src/views/header/header_example.js Co-authored-by: Chandler Prall <chandler.prall@gmail.com> * Update src-docs/src/views/header/header_alert.js Co-authored-by: Chandler Prall <chandler.prall@gmail.com> * Update src-docs/src/views/header/header_alert.js Co-authored-by: Chandler Prall <chandler.prall@gmail.com> * Update src-docs/src/views/header/header_animate.js Co-authored-by: Chandler Prall <chandler.prall@gmail.com> * Update src-docs/src/views/header/header_example.js Co-authored-by: Chandler Prall <chandler.prall@gmail.com> * renaming animate methods to euiAnimate * euiAnimate into header example * simplifying onClick closeFlyout * Update src-docs/src/views/header/header_example.js Co-authored-by: Caroline Horn <549577+cchaos@users.noreply.github.com> Co-authored-by: Chandler Prall <chandler.prall@gmail.com> Co-authored-by: cchaos <caroline.horn@elastic.co> Co-authored-by: Caroline Horn <549577+cchaos@users.noreply.github.com> * [Feature notification] Add EuiNotificationEvent (#4531) * Notification events * Looping through EuiNotificationEvent * Adding props table and more methods * wip * Renaming event notifications to event messages * onClickNoNewsTitles * Toggle mark as read/unread * Initial documentation * Improving examples * Fixing TS error * Refactored EuiEventNotification context menu item creation * improving data * Notifications data * Primary example buttons * Reset data * More docs * snippets * Adding tests * Better messages explanation * Better snippets * Small fixes * Missing prop explanation * Adding heading level * Adding flexible comp section * More a11y * Update src/components/notification/notification_event_messages.tsx Co-authored-by: Michail Yasonik <michail@yasonik.com> * Button messages ay11 * test * Better descriptions * Tests * Adding icon arial label into notifications feed example * i18n token * i18n token * i18 token again * primaryAction description * More description * Update src/components/notification/notification_event_messages.tsx Co-authored-by: Caroline Horn <549577+cchaos@users.noreply.github.com> * Update src/components/notification/notification_event_messages.tsx Co-authored-by: Caroline Horn <549577+cchaos@users.noreply.github.com> * Update src-docs/src/views/notification_event/notification_event_example.js Co-authored-by: Caroline Horn <549577+cchaos@users.noreply.github.com> * Update src-docs/src/views/notification_event/notification_event_example.js Co-authored-by: Caroline Horn <549577+cchaos@users.noreply.github.com> * Update src-docs/src/views/notification_event/notification_event_example.js Co-authored-by: Caroline Horn <549577+cchaos@users.noreply.github.com> * Update src-docs/src/views/notification_event/notification_event_example.js Co-authored-by: Caroline Horn <549577+cchaos@users.noreply.github.com> * Update src-docs/src/views/notification_event/notification_event_example.js Co-authored-by: Caroline Horn <549577+cchaos@users.noreply.github.com> * Addressing PR review * More improvements * Adding comment * Excluding `eventName` at top level and changing title vs aria-label on read button * Fix title * Fixing tests and removing evenName from docs usage * Fixing icon margin * More tests and small fixes * Moving read button from event meta comp to event comp * Split primaryAction and primaryActionProps * Exposing EuiPrimaryActionProps in props table * Update src-docs/src/views/notification_event/notification_event_example.js Co-authored-by: Caroline Horn <549577+cchaos@users.noreply.github.com> * Update src-docs/src/views/notification_event/notification_event_example.js Co-authored-by: Caroline Horn <549577+cchaos@users.noreply.github.com> * Update src-docs/src/views/notification_event/notification_event_example.js Co-authored-by: Caroline Horn <549577+cchaos@users.noreply.github.com> * Update src-docs/src/views/notification_event/notification_event_example.js Co-authored-by: Caroline Horn <549577+cchaos@users.noreply.github.com> * Update src-docs/src/views/notification_event/notification_event_example.js Co-authored-by: Caroline Horn <549577+cchaos@users.noreply.github.com> * Update src-docs/src/views/notification_event/notification_event_example.js Co-authored-by: Caroline Horn <549577+cchaos@users.noreply.github.com> * Update src-docs/src/views/notification_event/notification_event_example.js Co-authored-by: Caroline Horn <549577+cchaos@users.noreply.github.com> * Messages length and more * Fixing data test subj * Renaming tsx examples to js Co-authored-by: Chandler Prall <chandler.prall@gmail.com> Co-authored-by: Michail Yasonik <michail@yasonik.com> Co-authored-by: Caroline Horn <549577+cchaos@users.noreply.github.com> Co-authored-by: cchaos <caroline.horn@elastic.co> * Adding CL * Fixing CL a11y issue * Reverse logic in the ternary operator * Removed unecessary optional types in EuiNotificationEventMeta Co-authored-by: Caroline Horn <549577+cchaos@users.noreply.github.com> Co-authored-by: Chandler Prall <chandler.prall@gmail.com> Co-authored-by: cchaos <caroline.horn@elastic.co> Co-authored-by: Michail Yasonik <michail@yasonik.com>

Adding animation and hasBackground props to EuiHeaderSectionItemButton

915a2d1

A few improvements

14d5b51

Docs props

cc5c874

elizabetdev commented Jan 15, 2021

View reviewed changes

src/components/header/header_section/header_section_item_button.tsx Show resolved Hide resolved

elizabetdev marked this pull request as ready for review January 15, 2021 19:38

elizabetdev requested review from cchaos and chandlerprall January 15, 2021 19:38

cchaos suggested changes Jan 15, 2021

View reviewed changes

Removing hasBackground prop

4f5c09f

elizabetdev changed the title ~~[Feature Notification] Adding animation and hasBackground props to EuiHeaderSectionItemButton~~ [Feature Notification] Adding animation prop to EuiHeaderSectionItemButton Jan 18, 2021

Using JS do hide and show on xs breakpoints

e020d7a

Adding a snippet

c6f7bc3

chandlerprall and others added 2 commits January 29, 2021 14:39

refactor animation prop into a ref method

41a54c8

Merge pull request #8 from chandlerprall/notification_header_button_m…

e4b21e1

…ethod refactor animation prop into a ref method

elizabetdev added 2 commits February 1, 2021 12:48

Snapshots. Trigger animation comments.

d5923d3

Deleting unused css animation

4d3c970

elizabetdev commented Feb 1, 2021

View reviewed changes

src-docs/src/views/header/header_alert.js Outdated Show resolved Hide resolved

elizabetdev commented Feb 1, 2021

View reviewed changes

src-docs/src/views/header/header_alert.js Outdated Show resolved Hide resolved

triggerAnimation explanation

8253863

elizabetdev commented Feb 1, 2021

View reviewed changes

Cleaned up some logic and created a docs example specifically for the…

46a3d77

… notifications

cchaos reviewed Feb 8, 2021

View reviewed changes

Merge pull request #11 from cchaos/notification_header_button

f88a657

Cleaned up some logic and created a docs example specifically for the notifications

re-enable the animation test

aba5d8e

chandlerprall suggested changes Feb 10, 2021

View reviewed changes

elizabetdev and others added 8 commits February 10, 2021 18:44

Merge pull request #12 from chandlerprall/notification_header_button_…

d13e053

…test Re-enable the notification header button animation test

Update src-docs/src/views/header/header_animate.js

f3a8851

Co-authored-by: Chandler Prall <chandler.prall@gmail.com>

Update src-docs/src/views/header/header_example.js

733ae30

Co-authored-by: Chandler Prall <chandler.prall@gmail.com>

Update src-docs/src/views/header/header_alert.js

4634390

Co-authored-by: Chandler Prall <chandler.prall@gmail.com>

Update src-docs/src/views/header/header_alert.js

b4eb475

Co-authored-by: Chandler Prall <chandler.prall@gmail.com>

Update src-docs/src/views/header/header_animate.js

10736bd

Co-authored-by: Chandler Prall <chandler.prall@gmail.com>

Update src-docs/src/views/header/header_example.js

4b396ac

Co-authored-by: Chandler Prall <chandler.prall@gmail.com>

renaming animate methods to euiAnimate

36c586d

elizabetdev added 2 commits February 10, 2021 19:42

euiAnimate into header example

67d1c48

simplifying onClick closeFlyout

c5b041e

chandlerprall approved these changes Feb 10, 2021

View reviewed changes

elizabetdev requested a review from cchaos February 10, 2021 22:01

cchaos approved these changes Feb 11, 2021

View reviewed changes

src-docs/src/views/header/header_example.js Outdated Show resolved Hide resolved

Update src-docs/src/views/header/header_example.js

d431074

Co-authored-by: Caroline Horn <549577+cchaos@users.noreply.github.com>

elizabetdev merged commit a8da6d5 into elastic:feature/notification Feb 11, 2021

elizabetdev mentioned this pull request Feb 11, 2021

[Feature branch] Notification #4513

Merged

10 tasks

elizabetdev changed the title ~~[Feature Notification] Adding animation prop to EuiHeaderSectionItemButton~~ [Feature Notification] Add a euiAnimation method on the EuiHeaderSectionItemButton ref Feb 11, 2021

Conversation

elizabetdev commented Jan 14, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Checklist

Uh oh!

kibanamachine commented Jan 14, 2021

Uh oh!

kibanamachine commented Jan 14, 2021

Uh oh!

Uh oh!

kibanamachine commented Jan 15, 2021

Uh oh!

cchaos left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kibanamachine commented Jan 18, 2021

Uh oh!

kibanamachine commented Jan 18, 2021

Uh oh!

kibanamachine commented Jan 19, 2021

Uh oh!

kibanamachine commented Feb 1, 2021

Uh oh!

Uh oh!

Uh oh!

kibanamachine commented Feb 1, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kibanamachine commented Feb 1, 2021

Uh oh!

cchaos left a comment

Choose a reason for hiding this comment

Uh oh!

elizabetdev commented Feb 10, 2021

Uh oh!

kibanamachine commented Feb 10, 2021

Uh oh!

chandlerprall left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kibanamachine commented Feb 10, 2021

Uh oh!

kibanamachine commented Feb 10, 2021

Uh oh!

chandlerprall left a comment

Choose a reason for hiding this comment

Uh oh!

cchaos left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

kibanamachine commented Feb 11, 2021

Uh oh!

kibanamachine commented Feb 11, 2021

Uh oh!

elizabetdev commented Jan 14, 2021 •

edited

Loading