Make page titles more consistent for Overview, Alerts, Rules, Rule Detail and Cases pages#146150
Conversation
CoenWarmer
changed the title
CoenWarmer
added
v8.6.0
release_note:skip
formgeist
left a comment
formgeist
left a comment
There was a problem hiding this comment.
LGTM 👍 Thanks for fixing these inconsistencies
💚 Build Succeeded
Metrics [docs]Async chunks
Unknown metric groupsESLint disabled in files
ESLint disabled line counts
Total ESLint disabled count
History
To update your PR or re-run it, just comment with: |
| /> | ||
| </EuiButton> | ||
| ), | ||
| <EuiButton |
There was a problem hiding this comment.
This reduces the 'pop in' effect on this page where buttons suddenly jump to the left
There was a problem hiding this comment.
I like this improvement, just wondering if you checked with @maciejforcone whether disabling the button is the approach that we use in Kibana or do we need to use a loader for the title instead until we have the authorization information. (For the cases that the user actually doesn't have access)
There was a problem hiding this comment.
I checked how it works in Rules. So we do show "manage rules" always, but when user gets there and has no permissions to Create rule - then we don't show the button. But for example in Monitor view we show the button, but later in form we show the message "permissions needed".
For some actions we also show disabled button
So looks like we don't have a common pattern for that yet. I'd stick with what we have for now - showing the button, and then in form we show the info of missing permissions. And in case when there is instant action like this toggle:
the button can be disabled, with missing permissions info on hover.
There was a problem hiding this comment.
Ok, @maciejforcone agrees that a less jumpy UI is better, so the fix in this PR is good. @maryam-saeidi hope this answers your question.
There was a problem hiding this comment.
And to the question of loader vs element - less jumps is always better, so I like how @CoenWarmer solved that.
There was a problem hiding this comment.
@maciejforcone Thanks for the input, the loader option was for the whole page/title (not the button). I think I saw a similar approach on the cases page if I am not mistaken, so it will also not be jumpy. But if we use disable approach, that's also a good option.
| /> | ||
| </EuiButton> | ||
| ), | ||
| <EuiButton |
There was a problem hiding this comment.
I like this improvement, just wondering if you checked with @maciejforcone whether disabling the button is the approach that we use in Kibana or do we need to use a loader for the title instead until we have the authorization information. (For the cases that the user actually doesn't have access)
|
|
||
| return ( | ||
| <EuiFlexGroup wrap gutterSize="s" justifyContent="flexEnd"> | ||
| <EuiFlexItem grow={1}> |
| rangeTo={relativeEnd} | ||
| refreshInterval={refreshInterval} | ||
| refreshPaused={refreshPaused} | ||
| width="auto" |
There was a problem hiding this comment.
What is this change about?
There was a problem hiding this comment.
Is this change intentional?
| Before | After |
|---|---|
 |
 |
There was a problem hiding this comment.
I think the new Unified Search bar also employs this to make the size of the date picker component dynamic to the displayed value or label. I'm fine with moving in this direction, and we'll probably have to pick this up in other apps later on (if they're not already moving to the new Unified Search bar).
There was a problem hiding this comment.
Interesting, may I ask what was the reasoning behind it?
There was a problem hiding this comment.
@maryam-saeidi To save space in context of its surrounding elements, for instance the Unified Search bar date picker changes width depending on the value or content within.
The previous date picker in the Observability overview page had a fixed size which was the largest width possible (when the date range would be absolute date formats) which is unnecessary for most use-cases.
CoenWarmer
deleted the
bug/142859-inconsistent-page-title-styling-new
branch
…tail and Cases pages (elastic#146150) Closes elastic#142859. ## Summary This PR updates the Observability Overview, Alerts, Rules, Rule Details and Cases pages so their headers are more consistent with one another. **Previously:** https://user-images.githubusercontent.com/535564/203364454-35002377-d23a-4327-91b0-d1f473869f6a.mov **With this PR:** https://user-images.githubusercontent.com/535564/203364851-9afd2950-0fba-4f6b-ad52-0b81da77451a.mov (cherry picked from commit b3c022d)
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation |
* main: Make page titles more consistent for Overview, Alerts, Rules, Rule Detail and Cases pages (elastic#146150) [Files] Delay `<Image/>` blurhash reveal and handle blurhash errors (elastic#146159)
kibanamachine
added
the
backport missing
|
Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync. |
1 similar comment
|
Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync. |
|
Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync. |
2 similar comments
|
Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync. |
|
Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync. |
…ule Detail and Cases pages (#146150) (#146807) # Backport This will backport the following commits from `main` to `8.6`: - [Make page titles more consistent for Overview, Alerts, Rules, Rule Detail and Cases pages (#146150)](#146150) <!--- Backport version: 8.9.7 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT [{"author":{"name":"Coen Warmer","email":"coen.warmer@gmail.com"},"sourceCommit":{"committedDate":"2022-11-24T12:11:28Z","message":"Make page titles more consistent for Overview, Alerts, Rules, Rule Detail and Cases pages (#146150)\n\nCloses https://github.com/elastic/kibana/issues/142859.\r\n\r\n## Summary\r\n\r\nThis PR updates the Observability Overview, Alerts, Rules, Rule Details\r\nand Cases pages so their headers are more consistent with one another.\r\n\r\n**Previously:**\r\n\r\n\r\nhttps://user-images.githubusercontent.com/535564/203364454-35002377-d23a-4327-91b0-d1f473869f6a.mov\r\n\r\n**With this PR:**\r\n\r\n\r\nhttps://user-images.githubusercontent.com/535564/203364851-9afd2950-0fba-4f6b-ad52-0b81da77451a.mov","sha":"b3c022dc05fbe9f941627f526ba301cb1c29eefd","branchLabelMapping":{"^v8.7.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","v8.6.0","v8.7.0"],"number":146150,"url":"https://github.com/elastic/kibana/pull/146150","mergeCommit":{"message":"Make page titles more consistent for Overview, Alerts, Rules, Rule Detail and Cases pages (#146150)\n\nCloses https://github.com/elastic/kibana/issues/142859.\r\n\r\n## Summary\r\n\r\nThis PR updates the Observability Overview, Alerts, Rules, Rule Details\r\nand Cases pages so their headers are more consistent with one another.\r\n\r\n**Previously:**\r\n\r\n\r\nhttps://user-images.githubusercontent.com/535564/203364454-35002377-d23a-4327-91b0-d1f473869f6a.mov\r\n\r\n**With this PR:**\r\n\r\n\r\nhttps://user-images.githubusercontent.com/535564/203364851-9afd2950-0fba-4f6b-ad52-0b81da77451a.mov","sha":"b3c022dc05fbe9f941627f526ba301cb1c29eefd"}},"sourceBranch":"main","suggestedTargetBranches":["8.6"],"targetPullRequestStates":[{"branch":"8.6","label":"v8.6.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.7.0","labelRegex":"^v8.7.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/146150","number":146150,"mergeCommit":{"message":"Make page titles more consistent for Overview, Alerts, Rules, Rule Detail and Cases pages (#146150)\n\nCloses https://github.com/elastic/kibana/issues/142859.\r\n\r\n## Summary\r\n\r\nThis PR updates the Observability Overview, Alerts, Rules, Rule Details\r\nand Cases pages so their headers are more consistent with one another.\r\n\r\n**Previously:**\r\n\r\n\r\nhttps://user-images.githubusercontent.com/535564/203364454-35002377-d23a-4327-91b0-d1f473869f6a.mov\r\n\r\n**With this PR:**\r\n\r\n\r\nhttps://user-images.githubusercontent.com/535564/203364851-9afd2950-0fba-4f6b-ad52-0b81da77451a.mov","sha":"b3c022dc05fbe9f941627f526ba301cb1c29eefd"}}]}] BACKPORT--> Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
kibanamachine
removed
the
backport missing
8 participants
Closes #142859.
Summary
This PR updates the Observability Overview, Alerts, Rules, Rule Details and Cases pages so their headers are more consistent with one another.
Previously:
203355680-f6f88a6f-7fe8-4653-aeab-f464e0d90414.mov
With this PR:
Screen.Recording.2022-11-22.at.16.44.38.mov