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

[Uptime][Monitor Management] Add locations column to the monitor management table #121454

Merged
merged 1 commit into from
Dec 21, 2021
Merged

[Uptime][Monitor Management] Add locations column to the monitor management table #121454

merged 1 commit into from
Dec 21, 2021

Conversation

awahab07
Copy link
Contributor

@awahab07 awahab07 commented Dec 16, 2021 •
edited
Loading

Closes #121006

Summary

Adds Locations column to the monitor managements monitors list table.

  • Will show 3 locations at most initially and will trim others
  • A button indicating number of trimmed locations will reveal all the locations in the column

Screenshots:
121006-Add-Locations-To-Monitor-Management-table

Videos:
https://user-images.githubusercontent.com/2748376/146446606-77853eab-0472-4681-9995-dd74671d82a7.mov

How to test this PR locally

  • Go through this and enable monitor management.
  • Add or Edit a monitor and confirm the PR implements the said behavior.

Checklist

Delete any items that are not applicable to this PR.

For maintainers

@awahab07 awahab07 added v8.0.0 Team:Uptime - DEPRECATED Synthetics & RUM sub-team of Application Observability release_note:skip Skip the PR/issue when compiling release notes auto-backport Deprecated - use backport:version if exact versions are needed v8.1.0 labels Dec 16, 2021
@awahab07 awahab07 force-pushed the 121006-Add-locations-to-monitor-mangement-table branch from 030460c to 8b819cf Compare December 20, 2021 18:40
@awahab07 awahab07 force-pushed the 121006-Add-locations-to-monitor-mangement-table branch from 8b819cf to 89a1365 Compare December 21, 2021 18:29
@awahab07 awahab07 marked this pull request as ready for review December 21, 2021 18:30
@awahab07 awahab07 requested a review from a team as a code owner December 21, 2021 18:30
@elasticmachine
Copy link
Contributor

Pinging @elastic/uptime (Team:uptime)

dominiqueclarke
dominiqueclarke approved these changes Dec 21, 2021
View reviewed changes
Copy link
Contributor

@dominiqueclarke dominiqueclarke left a comment

Choose a reason for hiding this comment

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

LGTM. We might want to limit the width of certain columns, including locations and tags, so that tags and locations stack rather than taking up extra space. I hardcoded extra locations to see this behavior.

Monitor management
Screen Shot 2021-12-21 at 2 09 36 PM
Screen Shot 2021-12-21 at 2 15 04 PM

Uptime
Screen Shot 2021-12-21 at 2 10 00 PM

x-pack/plugins/uptime/public/components/monitor_management/monitor_list/monitor_list.tsx
@@ -57,7 +59,9 @@ export const MonitorManagementList = ({
const columns = [
{
align: 'left' as const,
name: 'Monitor name',
name: i18n.translate('xpack.uptime.monitorManagement.monitorList.monitorName', {
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for catching these.

@dominiqueclarke
Copy link
Contributor

Why are tags and locations not aligned in your photo? Any way to ensure that both are vertically aligned center.

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
uptime 666 667 +1

Async chunks

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

id before after diff
uptime 675.9KB 677.4KB +1.4KB

History

  • 💔 Build #14330 failed 8b819cf151c925b8eae250b6cbb48aa73fec7f65
  • 💚 Build #13741 succeeded 030460c00b7986579234995d7cd643c5c8bbc5e1

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

@awahab07 awahab07 removed the v8.1.0 label Dec 21, 2021
@awahab07 awahab07 merged commit b49e8dd into elastic:main Dec 21, 2021
@kibanamachine
Copy link
Contributor

The following labels were identified as gaps in your version labels and will be added automatically:

  • v8.1.0

If any of these should not be on your pull request, please manually remove them.

@kibanamachine
Copy link
Contributor

💔 Backport failed

Status Branch Result
❌ 8.0 Commit could not be cherrypicked due to conflicts

To backport manually run:
node scripts/backport --pr 121454

@awahab07 awahab07 mentioned this pull request Dec 21, 2021
Closed
26 tasks
@awahab07 awahab07 removed the v8.0.0 label Dec 21, 2021
@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create backports run node scripts/backport --pr 121454 or prevent reminders by adding the backport:skip label.

@kibanamachine kibanamachine added backport missing Added to PRs automatically when the are determined to be missing a backport. backport:skip This commit does not require backporting and removed backport missing Added to PRs automatically when the are determined to be missing a backport. labels Dec 22, 2021
@shahzad31 shahzad31 removed the auto-backport Deprecated - use backport:version if exact versions are needed label Jan 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dominiqueclarke dominiqueclarke dominiqueclarke approved these changes

Assignees
No one assigned
Labels
backport:skip This commit does not require backporting release_note:skip Skip the PR/issue when compiling release notes Team:Uptime - DEPRECATED Synthetics & RUM sub-team of Application Observability v8.1.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Uptime] Add Locations to the UI Monitor Management Table
6 participants
@awahab07 @elasticmachine @dominiqueclarke @kibana-ci @kibanamachine @shahzad31