Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[Logs UI] Log stream row rendering #60773

Merged
merged 17 commits into from
Mar 31, 2020
Merged

[Logs UI] Log stream row rendering #60773

merged 17 commits into from
Mar 31, 2020

Conversation

afgomez
Copy link
Contributor

@afgomez afgomez commented Mar 20, 2020 •
edited
Loading

Summary

Part of #58487. Implement changes in the log stream's row rendering.

  • Adjust the colors to match the new design.
  • Tweak overal rendering.
  • Replace the expand icon CTA with a menu. The menu will include only one CTA for now, but allows to include more items in the future (for view logs in context).

Checklist

Delete any items that are not applicable to this PR.

@afgomez afgomez added Feature:Logs UI Logs UI feature release_note:skip Skip the PR/issue when compiling release notes Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services v7.7.0 v8.0.0 labels Mar 20, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/logs-metrics-ui (Team:logs-metrics-ui)

@katrin-freihofner katrin-freihofner mentioned this pull request Mar 20, 2020
Closed
7 tasks
@afgomez afgomez changed the title log stream row rendering [Logs UI] Log stream row rendering Mar 20, 2020
@afgomez afgomez force-pushed the 58487-log-stream-row-rendering branch 2 times, most recently from 9667af0 to 39fbc1f Compare March 23, 2020 10:42
@afgomez afgomez marked this pull request as ready for review March 23, 2020 12:18
@afgomez afgomez requested a review from a team as a code owner March 23, 2020 12:18
Alejandro Fernández Gómez added 5 commits March 23, 2020 13:21
Use focusBackgroundColor for hovered lines
2fdfabc
Set hover styles using CSS
4675126 
This simplifies how the row renders, removing the need for each column
to have a `isHovered` prop to render the hovered styles.

Since now we rely in the browser, it also prevents some re-renders in
the columns.
Set highlighted styles in the row
5e3a20e 
Instead of applying the style for each individual column, apply them to
the containing row. It's easier to reason about, simplifies the
interface to the columns, and prevents rerendering.
Rename icon column to actions column
27708a3
Turn expand button into a menu
c948b9a
@afgomez afgomez force-pushed the 58487-log-stream-row-rendering branch from 17fb4bd to c948b9a Compare March 23, 2020 12:21
weltenwort
weltenwort reviewed Mar 25, 2020
View reviewed changes
Copy link
Member

@weltenwort weltenwort left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👀 Reviewing on behalf of @elastic/logs-metrics-ui...

weltenwort
weltenwort reviewed Mar 25, 2020
View reviewed changes
Copy link
Member

@weltenwort weltenwort left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks really good, I just left a few comments on the usability and component re-use.

x-pack/plugins/infra/public/components/logging/log_text_stream/log_entry_row.tsx Outdated Show resolved Hide resolved
x-pack/plugins/infra/public/components/logging/log_text_stream/log_entry_actions_column.tsx Outdated Show resolved Hide resolved
x-pack/plugins/infra/public/components/logging/log_text_stream/log_entry_actions_column.tsx Outdated Show resolved Hide resolved
x-pack/plugins/infra/public/components/logging/log_text_stream/log_entry_actions_column.tsx Outdated Show resolved Hide resolved
@afgomez
Copy link
Contributor Author

afgomez commented Mar 27, 2020

@elasticmachine merge upstream

weltenwort
weltenwort approved these changes Mar 30, 2020
View reviewed changes
Copy link
Member

@weltenwort weltenwort left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems to work well, nice job 👍

x-pack/plugins/infra/public/components/logging/log_text_stream/log_entry_actions_column.tsx Show resolved Hide resolved
@kibanamachine
Copy link
Contributor

💚 Build Succeeded

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@afgomez afgomez merged commit 4632f3e into elastic:master Mar 31, 2020
@afgomez afgomez deleted the 58487-log-stream-row-rendering branch March 31, 2020 11:12
@afgomez afgomez mentioned this pull request Mar 31, 2020
Merged
afgomez pushed a commit that referenced this pull request Mar 31, 2020
[7.x] [Logs UI] Log stream row rendering (#60773) (#61953)
d456943
gmmorris added a commit to gmmorris/kibana that referenced this pull request Mar 31, 2020
@gmmorris
Merge remote-tracking branch 'upstream/master' into alerting/start-trial
2d80df2 
* upstream/master: (69 commits)
  Adding PagerDuty icon to connectors cards (elastic#60805)
  Fix drag and drop flakiness (elastic#61993)
  Grok debugger migration (elastic#60658)
  Endpoint: Fix resolver SVG position issue (elastic#61886)
  [SIEM] version 7.7 rule import (elastic#61903)
  Added styles to make combobox list items wider for alerting flyout (elastic#61894)
  [UA] Tight worker loop can cause high CPU usage (elastic#60950)
  [ML] DF Analytics results table: use index pattern field format if one exists (elastic#61709)
  [ML] Catching unknown index pattern errors (elastic#61935)
  [Discover] Deangularize and euificate sidebar  (elastic#47559)
  Endpoint: Add ts-node dev dependency (elastic#61884)
  Add an onBlur handler for the kuery bar. Only resubmit when input changes. (elastic#61901)
  [ML] Handle Empty Partition Field Values in Single Metric Viewer (elastic#61649)
  Auto interval on date histogram is getting displayed as timestamp per… (elastic#59171)
  [Maps] Explicitly pass fetch function to ems-client (elastic#61846)
  [SIEM][CASE] Fix aria-labels and translations (elastic#61670)
  [ML] Settings: Increase number of items that can be paged in calendars and filters lists (elastic#61842)
  [EPM] update epm filepath route (elastic#61910)
  APM] Set ignore_above to 1024 for telemetry saved object (elastic#61732)
  [Logs UI] Log stream row rendering (elastic#60773)
  ...
gmmorris added a commit to gmmorris/kibana that referenced this pull request Apr 1, 2020
@gmmorris
Merge branch 'master' into event-log/query-support
a0e1dcd 
* master: (64 commits)
  Adding PagerDuty icon to connectors cards (elastic#60805)
  Fix drag and drop flakiness (elastic#61993)
  Grok debugger migration (elastic#60658)
  Endpoint: Fix resolver SVG position issue (elastic#61886)
  [SIEM] version 7.7 rule import (elastic#61903)
  Added styles to make combobox list items wider for alerting flyout (elastic#61894)
  [UA] Tight worker loop can cause high CPU usage (elastic#60950)
  [ML] DF Analytics results table: use index pattern field format if one exists (elastic#61709)
  [ML] Catching unknown index pattern errors (elastic#61935)
  [Discover] Deangularize and euificate sidebar  (elastic#47559)
  Endpoint: Add ts-node dev dependency (elastic#61884)
  Add an onBlur handler for the kuery bar. Only resubmit when input changes. (elastic#61901)
  [ML] Handle Empty Partition Field Values in Single Metric Viewer (elastic#61649)
  Auto interval on date histogram is getting displayed as timestamp per… (elastic#59171)
  [Maps] Explicitly pass fetch function to ems-client (elastic#61846)
  [SIEM][CASE] Fix aria-labels and translations (elastic#61670)
  [ML] Settings: Increase number of items that can be paged in calendars and filters lists (elastic#61842)
  [EPM] update epm filepath route (elastic#61910)
  APM] Set ignore_above to 1024 for telemetry saved object (elastic#61732)
  [Logs UI] Log stream row rendering (elastic#60773)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@weltenwort weltenwort weltenwort approved these changes

Assignees
No one assigned
Labels
Feature:Logs UI Logs UI feature release_note:skip Skip the PR/issue when compiling release notes Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services v7.7.0 v8.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@afgomez @elasticmachine @kibanamachine @weltenwort