[Feature branch] Notification by elizabetdev · Pull Request #4513 · elastic/eui

elizabetdev · 2021-02-11T16:57:41Z

Summary

This PR add a new component called EuiNotification Event and a new euiAnimation() method on the EuiHeaderSectionItemButton ref.

This work is essential to achieve a notification template similar to MVP Prototype.

Feature spec

You can find the feature spec: here.

Build Preview

You can find the preview here: here

Changelog and PR's

As part of #4433

Add notification event meta

As part of #4424

Add euiAnimation() method on the EuiHeaderSectionItemButton ref

As part of #4531

Add EuiNotificationEvent

Checklist

* 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>

…utton (#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>

kibanamachine · 2021-02-11T17:21:11Z

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

kibanamachine · 2021-02-15T17:08:31Z

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

* 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>

kibanamachine · 2021-03-09T16:46:22Z

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

kibanamachine · 2021-03-10T15:52:18Z

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

kibanamachine · 2021-03-11T18:01:45Z

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

kibanamachine · 2021-03-11T19:24:27Z

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

cchaos

🎉 Great job!! Can't wait to see this in action.

mshustov · 2021-03-15T21:08:45Z

src/components/notification/notification_event.tsx

+            onOpenContextMenu ? () => onOpenContextMenu(id) : undefined
+          }
+          eventName={title}
+          onRead={() => onRead?.(id, isRead!)}


Probably isRead should be required then? or pass an optional flag?

In case you don't pass isRead, the read button won't appear. So that's why we can't make it required.

We discussed if it was possible for the MVP to have a way to save the read states and because was something that we were not completely sure it was decided to live it in open.

Actually, I deleted this line onRead={() => onRead?.(id, isRead!)}. After a refactor in #4433 the read button was left outside of the EuiNotificationEventMeta and this line was doing nothing.

Thanks, @restrry . It was a good catch!

mshustov · 2021-03-15T21:10:57Z

src/components/notification/notification_event.tsx

+          iconType={iconType}
+          iconAriaLabel={iconAriaLabel}
+          time={time}
+          onOpenContextMenu={


nit: onOpenContextMenu={onOpenContextMenu}
EuiNotificationEventMeta declares it as an optional field.

When I try to do this TS starts complaining:

src/components/notification/notification_event_read_button.tsx

mshustov · 2021-03-15T21:19:23Z

src/components/notification/notification_event_messages.tsx

+  /*
+   * An array of strings that get individually wrapped in `<p>` tags
+   */
+  messages: string[];


Do we require messages to be already translated? Let's mention it in the comment.
PS. the same for other text props.

In EUI we assume translating a string is a decision from the consumer app.

mshustov · 2021-03-15T21:23:35Z

src/components/notification/notification_event.tsx

+        />
+
+        {onClickTitle ? (
+          <EuiLink onClick={() => onClickTitle(id)} {...titleProps}>


Can it be configured with href?

No. You need to create a logic to redirect to a page or external link.

mshustov · 2021-03-15T21:32:01Z

src/components/notification/notification_event.tsx

+        <div className="euiNotificationEvent__readButton">
+          <EuiNotificationEventReadButton
+            isRead={isRead}
+            onClick={() => onRead?.(id, isRead)}


Should we verify whether it's defined?

It's being verified. 🎉

I checked with @chandlerprall if the onRead?. was verifying that.

kibanamachine · 2021-03-16T13:44:57Z

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

kibanamachine · 2021-03-16T15:14:04Z

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

elizabetdev · 2021-03-16T15:32:07Z

Thanks, @restrry. I went through your comments and improved a few things. 🎉

If you have more suggestions don't hesitate to open an issue.

elizabetdev and others added 2 commits February 11, 2021 16:46

elizabetdev mentioned this pull request Feb 15, 2021

[Feature Notification] Add EuiNotificationEventMeta #4433

Merged

10 tasks

Merge remote-tracking branch 'upstream/master' into feature/notification

b0842d0

Merge branch 'master' into feature/notification

c4374b7

Adding CL

848e656

elizabetdev marked this pull request as ready for review March 11, 2021 17:51

Fixing CL a11y issue

e928712

cchaos approved these changes Mar 11, 2021

View reviewed changes

elizabetdev requested a review from mshustov March 11, 2021 20:01

mshustov approved these changes Mar 15, 2021

View reviewed changes

mshustov reviewed Mar 15, 2021

View reviewed changes

src/components/notification/notification_event_read_button.tsx Outdated Show resolved Hide resolved

mshustov reviewed Mar 15, 2021

View reviewed changes

elizabetdev added 3 commits March 16, 2021 12:34

Reverse logic in the ternary operator

748b4d0

Merge remote-tracking branch 'upstream/master' into feature/notification

59c7614

Removed unecessary optional types in EuiNotificationEventMeta

4155c62

Merge remote-tracking branch 'upstream/master' into feature/notification

e7db5ff

elizabetdev merged commit 1d6fec1 into master Mar 16, 2021

thompsongl mentioned this pull request Apr 7, 2021

Upgrade EUI to v32.0.4 elastic/kibana#96459

Merged

thompsongl deleted the feature/notification branch November 1, 2021 18:52

Conversation

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

Uh oh!

Summary

Feature spec

Build Preview

Changelog and PR's

Checklist

Uh oh!

kibanamachine commented Feb 11, 2021

Uh oh!

kibanamachine commented Feb 15, 2021

Uh oh!

kibanamachine commented Mar 9, 2021

Uh oh!

kibanamachine commented Mar 10, 2021

Uh oh!

kibanamachine commented Mar 11, 2021

Uh oh!

kibanamachine commented Mar 11, 2021

Uh oh!

cchaos left a comment

Choose a reason for hiding this comment

Uh oh!

mshustov Mar 15, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

mshustov Mar 15, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

mshustov Mar 15, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kibanamachine commented Mar 16, 2021

Uh oh!

kibanamachine commented Mar 16, 2021

Uh oh!

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

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

elizabetdev commented Feb 11, 2021 •

edited

Loading

mshustov Mar 15, 2021 •

edited

Loading

mshustov Mar 15, 2021 •

edited

Loading

mshustov Mar 15, 2021 •

edited

Loading

elizabetdev commented Mar 16, 2021 •

edited

Loading