Skip to content

[Stack Monitoring] Visual refresh changes#204258

Merged
weronikaolejniczak merged 35 commits intoelastic:mainfrom
weronikaolejniczak:feat/monitoring-visual-refresh
Jan 6, 2025
Merged

[Stack Monitoring] Visual refresh changes#204258
weronikaolejniczak merged 35 commits intoelastic:mainfrom
weronikaolejniczak:feat/monitoring-visual-refresh

Conversation

@weronikaolejniczak
Copy link
Copy Markdown
Contributor

@weronikaolejniczak weronikaolejniczak commented Dec 13, 2024 •
edited
Loading

Summary

This PR introduces changes to x-pack/plugins/monitoring necessary for the Visual Refresh project (#199715):

  • replacing euiThemeVars with euiTheme context
  • replacing old tokens with euiTheme
  • making sure all color palette functions are run within the context of the EuiProvider

Additionally:

  • I migrated Sass to @emotion/react
  • I migrated euiStyled to @emotion/react
  • I extended emotion.d.ts in tsconfig.json for typing of the EUI theme

closes #8228

QA

We need to test the critical paths in the Stack monitoring, paying close attention to:

  • color palette, visibility and contrast ratio of elements in Amsterdam / Borealis

Specific paths:

  • Monitoring time-series "Zoom out" button hover behavior - x-pack/plugins/monitoring/public/components/chart/monitoring_timeseries_container.tsx
  • Shard allocation (especially color mapping with shard type and status):
    • x-pack/plugins/monitoring/public/components/elasticsearch/shard_allocation/components/assigned.js
    • x-pack/plugins/monitoring/public/components/elasticsearch/shard_allocation/components/shard.js
  • Kuery bar suggestions and autocomplete field:
    • x-pack/plugins/monitoring/public/components/kuery_bar/autocomplete_field.tsx
    • x-pack/plugins/monitoring/public/components/kuery_bar/suggestion_item.tsx

Checklist

  • Unit or functional tests were updated
  • The PR description includes the appropriate Release Notes section, and the correct release_note:* label is applied per the guidelines

@weronikaolejniczak weronikaolejniczak added EUI Visual Refresh backport:skip This PR does not require backporting labels Dec 13, 2024
@weronikaolejniczak weronikaolejniczak marked this pull request as ready for review December 16, 2024 12:08
@weronikaolejniczak weronikaolejniczak requested review from a team as code owners December 16, 2024 12:08
@weronikaolejniczak weronikaolejniczak added the release_note:skip Skip the PR/issue when compiling release notes label Dec 16, 2024
mgadewoll
mgadewoll requested changes Dec 16, 2024
View reviewed changes
@andreadelrio andreadelrio changed the title Feat/monitoring visual refresh [Stack Monitoring] Visual refresh changes Dec 16, 2024
@consulthys
Copy link
Copy Markdown
Contributor

consulthys commented Dec 17, 2024 •
edited
Loading

Thank you everyone for helping us on this, truly appreciated!

I've made a first test run with Amsterdam to verify that everything was still looking "as usual". I've been through all screens and I have nothing to report, everything looks great!

I'm not able to start Kibana with the Borealis theme (errors out), I've shared some details here so I'll do another test run with Borealis once we get that sorted out.

@weronikaolejniczak
Copy link
Copy Markdown
Contributor Author

weronikaolejniczak commented Dec 17, 2024

/ci

@patpascal
Copy link
Copy Markdown
Contributor

patpascal commented Dec 17, 2024

All LGTM -- I just noticed a badge (Replica) that I'm not sure should be success as a positive reinforcement , or secondary accent - leaving it here!
Screenshot 2024-12-17 at 11 50 32

@consulthys
Copy link
Copy Markdown
Contributor

consulthys commented Dec 17, 2024

All LGTM -- I just noticed a badge (Replica) that I'm not sure should be success as a positive reinforcement , or secondary accent - leaving it here!

Totally agree with @patpscal it can definitely be secondary accent

@weronikaolejniczak
Copy link
Copy Markdown
Contributor Author

weronikaolejniczak commented Dec 17, 2024

@patpscal @consulthys great catch! Just fyi, accentSecondary is removed for the EuiBadge component. We can pass in the color prop anything that we want, it doesn't have to be danger | warning | accent | default | primary | success.

Screenshot 2024-12-17 at 13 36 50

@consulthys
Copy link
Copy Markdown
Contributor

consulthys commented Dec 17, 2024

@patpscal @consulthys great catch! Just fyi, accentSecondary is removed for the EuiBadge component. We can pass in the color prop anything that we want, it doesn't have to be danger | warning | accent | default | primary | success.

Thanks @weronikaolejniczak I think instead of success we just need a green-ish color

@weronikaolejniczak
Copy link
Copy Markdown
Contributor Author

weronikaolejniczak commented Dec 17, 2024 •
edited
Loading

@andreadelrio @mgadewoll @patpscal @consulthys thank you so much for the testing and suggestions! 💚 I updated the PR with the changes.

Let me know the final decision on those legend badge colors and I'd appreciate a double-check of the paths I defined in the PR description (where I made riskier style changes).

Testing:

- ✅ Removing wrapping <div> (addressing: #204258 (comment))
File: x-pack/plugins/monitoring/public/components/beats/listing/listing.js
Path: Menu bar > Under "Management" > Stack Monitoring > Under "Beats" > Beats (localhost url)

Amsterdam Borealis
Screenshot 2024-12-17 at 14 18 30

- ✅ Removing wrapping <div> (addressing: #204258 (comment))
File: x-pack/plugins/monitoring/public/components/kibana/instances/instances.tsx
Path: Menu bar > Under "Management" > Stack Monitoring > Under "Kibana" > Instances (localhost url)

Amsterdam Borealis
Screenshot 2024-12-17 at 14 16 45

Warning

The issue with the Badge overlapping the next column is reproducible regardless of the changes made in this PR. If you have a ready solution, I'd appreciate it! Otherwise, let's leave it like so.

Untested paths (fyi @consulthys):

- Removing wrapping <div> (addressing: #204258 (comment))
File: x-pack/plugins/monitoring/public/components/logstash/listing/listing.js
Path: Menu bar > Under "Management" > Stack Monitoring > Under "Logstash" > Instances

- Removing wrapping <div> (addressing: #204258 (comment))
File: x-pack/plugins/monitoring/public/components/apm/instances/instances.tsx
Path: Menu bar > Under "Management" > Stack Monitoring > Under "APM server" > APM servers (localhost url)

- Removing !important flag (addressing: #204258 (comment))
File: x-pack/plugins/monitoring/public/components/logstash/pipeline_viewer/views/metric.tsx
Path: Menu bar > Under "Management" > Stack Monitoring > Under "Logstash" > Pipeline viewer

@andreadelrio
Copy link
Copy Markdown
Contributor

andreadelrio commented Dec 17, 2024 •
edited
Loading

@patpscal @consulthys great catch! Just fyi, accentSecondary is removed for the EuiBadge component. We can pass in the color prop anything that we want, it doesn't have to be danger | warning | accent | default | primary | success.

Thanks @weronikaolejniczak I think instead of success we just need a green-ish color

I think sticking with success is our best option here. No harm in using it.

consulthys
consulthys approved these changes Dec 18, 2024
View reviewed changes
Copy link
Copy Markdown
Contributor

@consulthys consulthys left a comment

Choose a reason for hiding this comment

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

Everything looks good on both Amsterdam and Borealis.
Thank you everyone again for helping us on this!

mgadewoll
mgadewoll approved these changes Dec 18, 2024
View reviewed changes
Copy link
Copy Markdown
Contributor

@mgadewoll mgadewoll left a comment

Choose a reason for hiding this comment

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

🚢 🐈‍⬛ Thanks for the additional changes! Great work on this 🎉
I just left two small non-blocking comments.

@weronikaolejniczak
Copy link
Copy Markdown
Contributor Author

weronikaolejniczak commented Dec 18, 2024

/ci

@weronikaolejniczak weronikaolejniczak requested a review from a team as a code owner December 26, 2024 11:41
@weronikaolejniczak weronikaolejniczak force-pushed the feat/monitoring-visual-refresh branch from 2e2db71 to a64a966 Compare December 26, 2024 12:09
@weronikaolejniczak
Copy link
Copy Markdown
Contributor Author

weronikaolejniczak commented Dec 29, 2024

@elasticmachine merge upstream

@weronikaolejniczak
Copy link
Copy Markdown
Contributor Author

weronikaolejniczak commented Jan 2, 2025

/ci

@weronikaolejniczak
Copy link
Copy Markdown
Contributor Author

weronikaolejniczak commented Jan 6, 2025

@elasticmachine merge upstream

@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Jan 6, 2025

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
monitoring 478 396 -82

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
monitoring 532.1KB 514.8KB -17.3KB

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
monitoring 25.5KB 25.3KB -217.0B
Unknown metric groups

async chunk count

id before after diff
monitoring 6 7 +1

ESLint disabled line counts

id before after diff
monitoring 25 26 +1

Total ESLint disabled count

id before after diff
monitoring 32 33 +1

History

@weronikaolejniczak weronikaolejniczak merged commit b95211b into elastic:main Jan 6, 2025
viduni94 pushed a commit to viduni94/kibana that referenced this pull request Jan 23, 2025
@weronikaolejniczak @kibanamachine
@elasticmachine @viduni94
[Stack Monitoring] Visual refresh changes (elastic#204258)
169144a 
## Summary

This PR introduces changes to `x-pack/plugins/monitoring` necessary for
the Visual Refresh project
(elastic#199715):

- replacing `euiThemeVars` with `euiTheme` context
- replacing old tokens with `euiTheme`
- making sure all color palette functions are run within the context of
the `EuiProvider`

Additionally:

- I migrated Sass to `@emotion/react`
- I migrated `euiStyled` to `@emotion/react`
- I extended `emotion.d.ts` in `tsconfig.json` for typing of the EUI
theme

closes [elastic#8228](elastic/eui#8228)

### QA

We need to test the critical paths in the Stack monitoring, paying close
attention to:

- [ ] color palette, visibility and contrast ratio of elements in
Amsterdam / Borealis

Specific paths:
- [ ] Monitoring time-series "Zoom out" button hover behavior -
`x-pack/plugins/monitoring/public/components/chart/monitoring_timeseries_container.tsx`
- [ ] Shard allocation (especially color mapping with shard type and
status):
- [ ]
`x-pack/plugins/monitoring/public/components/elasticsearch/shard_allocation/components/assigned.js`
- [ ]
`x-pack/plugins/monitoring/public/components/elasticsearch/shard_allocation/components/shard.js`
- [ ] Kuery bar suggestions and autocomplete field:
- [ ]
`x-pack/plugins/monitoring/public/components/kuery_bar/autocomplete_field.tsx`
- [ ]
`x-pack/plugins/monitoring/public/components/kuery_bar/suggestion_item.tsx`

### Checklist

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated
- [x] The PR description includes the appropriate Release Notes section,
and the correct `release_note:*` label is applied per the
[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)

---------

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
@weronikaolejniczak weronikaolejniczak deleted the feat/monitoring-visual-refresh branch February 10, 2026 10:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@consulthys consulthys consulthys approved these changes

@mgadewoll mgadewoll mgadewoll approved these changes

@patpascal patpascal patpascal approved these changes

Assignees

No one assigned

Labels

backport:skip This PR does not require backporting EUI Visual Refresh release_note:skip Skip the PR/issue when compiling release notes v9.0.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Implement Visual Refresh changes in Stack monitoring

7 participants

@weronikaolejniczak @consulthys @patpascal @andreadelrio @elasticmachine @mgadewoll @kibanamachine