[Uptime] Redesign/44541 new monitor list expanded row #46567
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…://github.com/shahzad31/kibana into redesign/44541--new-monitor-list-expanded-row
💔 Build Failed |
💔 Build Failed |
💔 Build Failed |
shahzad31
added
v8.0.0
Team:Uptime - DEPRECATED
|
Pinging @elastic/uptime (Team:uptime) |
shahzad31
changed the title
💚 Build Succeeded |
💔 Build Failed |
💚 Build Succeeded |
There was a problem hiding this comment.
Review
This looks great overall. I have a few UI-based concerns that I detail below. I think we should answer these questions before we merge so we have action items coming out of this patch.
Additionally I had a few questions and recommendations, but most of them are subjective. The code, as always, looks to be of good overall quality to me!
Most recent error
This isn't feedback for this PR per se but an enhancement I think would make the change more useful now that I'm seeing it, we should include a timestamp for most recent error, especially since we are showing this data for checks that have an Up status as well. If my monitor is up and I'm still being shown the most recent error, I'd like to know if it was 30s ago or 3 days ago.
I'm fine with creating an enhancement issue around this and adding it afterwards.
External linking
Is this needed in the expanded drawer? We already expose this in the parent table row:
Looking back on the design issue I don't see any mention of this fact, and I think it's probably because it didn't occur to any of us as we were all working of a screenshot that didn't show the drawer nested in the table.
x-pack/legacy/plugins/uptime/public/components/functional/filter_group/filter_popover.tsx
Outdated
Show resolved
Hide resolved
...uptime/public/components/functional/monitor_list/monitor_list_drawer/monitor_status_list.tsx
Outdated
Show resolved
Hide resolved
...uptime/public/components/functional/monitor_list/monitor_list_drawer/monitor_status_list.tsx
Outdated
Show resolved
Hide resolved
...uptime/public/components/functional/monitor_list/monitor_list_drawer/monitor_status_list.tsx
Outdated
Show resolved
Hide resolved
...uptime/public/components/functional/monitor_list/monitor_list_drawer/monitor_status_list.tsx
Outdated
Show resolved
Hide resolved
x-pack/legacy/plugins/uptime/server/lib/adapters/monitors/elasticsearch_monitors_adapter.ts
Show resolved
Hide resolved
x-pack/legacy/plugins/uptime/server/rest_api/monitors/monitors_details.ts
Show resolved
Hide resolved
|
@justinkambic i have taken care of all the coding feedback, and have create the enhancement issue #50981 for UI suggestion, i think we need some design feedback on that, i will ask katrin there. |
💚 Build Succeeded |
.../uptime/public/components/functional/monitor_list/monitor_list_drawer/monitor_status_row.tsx
Outdated
Show resolved
Hide resolved
...uptime/public/components/functional/monitor_list/monitor_list_drawer/monitor_status_list.tsx
Outdated
Show resolved
Hide resolved
💔 Build Failed |
💔 Build Failed |
💚 Build Succeeded |
shahzad31
added a commit
to shahzad31/kibana
that referenced
this pull request
Nov 27, 2019
* expanded row component * expanded row component * updated expand comp * update expander row * update expanded row * update graphql schema * update styling * added monitor detail route * added monitor state * added monitor details state * update drawer * update drawer * updated drawer components * update monitor drawer * update tests * revert some changes * revert some changes * revert some changes * update test * update drawer * added run time validation * update monitor status * added location icon * update locations * fix relative path * update unamelocation * updated snaps * update formatting * updated snaps * update snaps * improve code formatting * update snaps * updated translation formatting !! * update snaps * update i18n
shahzad31
added a commit
that referenced
this pull request
Nov 27, 2019
* expanded row component * expanded row component * updated expand comp * update expander row * update expanded row * update graphql schema * update styling * added monitor detail route * added monitor state * added monitor details state * update drawer * update drawer * updated drawer components * update monitor drawer * update tests * revert some changes * revert some changes * revert some changes * update test * update drawer * added run time validation * update monitor status * added location icon * update locations * fix relative path * update unamelocation * updated snaps * update formatting * updated snaps * update snaps * improve code formatting * update snaps * updated translation formatting !! * update snaps * update i18n
jloleysens
added a commit
to jloleysens/kibana
that referenced
this pull request
Nov 27, 2019
* upstream/7.x: Fix infinite redirect loop when multiple cookies are sent (elastic#50452) (elastic#51821) [Console] Proxy fallback (elastic#50185) (elastic#51814) Added endgame-* index and new heading 3 Elastic Endpoint SMP. (elastic#51071) (elastic#51828) [Maps] Added options to disable zoom, hide tool tips, widgets/overlays in embeddable maps (elastic#50663) (elastic#51811) Move errors and validate index pattern ⇒ NP (elastic#51805) (elastic#51831) [SIEM][Detection Engine] Adds ecs threat properties to rules (elastic#51782) (elastic#51827) [Lens] Remove client-side reference to server source code (elastic#51763) (elastic#51825) fixes drag and drop in tests (elastic#51806) (elastic#51813) [Uptime] Redesign/44541 new monitor list expanded row (elastic#46567) (elastic#51809) [7.x] [Telemetry] collector set to np (elastic#51618) (elastic#51787) [Uptime] added test for chart wrapper (elastic#50399) (elastic#51808) Expressions service fixes: better error and loading states handling (elastic#51183) (elastic#51800) Query String(Bar) Input - cleanup (elastic#51598) (elastic#51804) [ML] Adjust and re-enable categorization advanced wizard test (elastic#51005) (elastic#51017) fixes url state tests (elastic#51746) (elastic#51798) fixes browser field tests (elastic#51738) (elastic#51799) [Task Manager] Tests for the ability to run tasks of varying durations in parallel (elastic#51572) (elastic#51701) [ML] Fix anomaly detection test suite (elastic#51712) (elastic#51795) [SIEM] Fix Timeline drag and drop behavior (elastic#51558) (elastic#51793)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Fixes: #44541
Redesigned monitor list expanded row
Testing
Expanded row with call out for missing location config
Checklist
For maintainers