Improve URL drilldown authoring experience #197454
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
|
@elasticmachine merge upstream |
…Dosant/kibana into d/2024-10-23-url-drilldow-error-improve
Dosant
changed the title
Dosant
commented
Oct 24, 2024
| @@ -11,6 +11,7 @@ import type { SerializableRecord } from '@kbn/utility-types'; | |||
|
|
|||
| export type BaseActionConfig = SerializableRecord; | |||
|
|
|||
| // eslint-disable-next-line @typescript-eslint/consistent-type-definitions | |||
There was a problem hiding this comment.
I removed .eslintrc.json in that dir because the linting in IDE didn't work with it. I just put the overrides inline where needed
| @@ -9,48 +9,50 @@ | |||
|
|
|||
| import { i18n } from '@kbn/i18n'; | |||
|
|
|||
| export const txtUrlTemplatePlaceholder = i18n.translate( | |||
Dosant
added
release_note:skip
|
Pinging @elastic/appex-sharedux (Team:SharedUX) |
Dosant
added
release_note:fix
and removed
release_note:skip
|
@elasticmachine merge upstream |
|
@elasticmachine merge upstream |
💚 Build Succeeded
Metrics [docs]Async chunks
Page load bundle
Unknown metric groupsESLint disabled line counts
Total ESLint disabled count
History
|
nreese
approved these changes
Oct 31, 2024
eokoneyo
approved these changes
Nov 4, 2024
| }); | ||
| return <>{title}</>; | ||
|
|
||
| // eslint-disable-next-line react-hooks/exhaustive-deps |
There was a problem hiding this comment.
nit: I think it might be sufficient to check that within this hook the effect only runs when title is undefined since we only ever set it's value here, here by removing the need for this comment, wdyt?
There was a problem hiding this comment.
Looks like initial title is not undefined, but is a raw value :(
it makes the potential check a bit less clear and reliable .
|
@elasticmachine merge upstream |
|
Starting backport for target branches: 8.x |
kibanamachine
pushed a commit
to kibanamachine/kibana
that referenced
this pull request
Nov 4, 2024
## Summary close elastic#163642 close elastic#163641 As part of fix-it-week I picked up some of existing URL drilldown authoring issues hoping to improve it a bit with a low effort (we don't want to spend to much time on it). Current URL drilldown authoring experience is terrible mainly because there is no proper validation while creating the drilldown as we don't have the needed runtime context. In the initial version we had a preview but it was very limited and used "dummy" context and in some cases got in the way by blocking the "save" button for URLs that would have been valid in runtime. We simply removed the preview and validaiton on some point later, so you can create an URL drilldown only by trial and error. This is still the case in this PR, but it slightly improve the experience: Firstly, **ONLY IN EDIT MODE** instead of hidding "invalid" drilldowns, we're showing them now with an error. This helps to find broken drilldowns and address issues. fixes elastic#163641  This is far from ideal, but this is better from what we have now. As for the error UI I wanted to use EuiIconTip, but it doesn't work well when inside that context menu because tooltip is shown below the menu. I didn't want to change zIndex as it might cause regressions in other places, so I went for this inline error truncated after 3 lines and the whole error is shown in native browser tooltip when hovered. The error is also printed in console In addition to that I've slightly improved the form editor experience - Show simple non-blocking validation error (if URL is missing or the pattern doesn't look like an URL) - Add a help text about how to test and properly validate <img width="576" alt="Screenshot 2024-10-24 at 15 35 01" src="https://github.com/user-attachments/assets/248b5c03-d445-4b30-97a2-a0a9d6a055a4"> <img width="547" alt="Screenshot 2024-10-24 at 15 35 08" src="https://github.com/user-attachments/assets/c265ee1c-94ec-4238-85fb-fc90f069f3b4"> This is again, not ideal, but slighltly better then before (cherry picked from commit 014b956)
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation |
kibanamachine
added a commit
that referenced
this pull request
Nov 4, 2024
# Backport This will backport the following commits from `main` to `8.x`: - [Improve URL drilldown authoring experience (#197454)](#197454) <!--- Backport version: 9.4.3 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT [{"author":{"name":"Anton Dosov","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-11-04T14:16:11Z","message":"Improve URL drilldown authoring experience (#197454)\n\n## Summary\r\n\r\nclose https://github.com/elastic/kibana/issues/163642\r\nclose https://github.com/elastic/kibana/issues/163641\r\n\r\nAs part of fix-it-week I picked up some of existing URL drilldown\r\nauthoring issues hoping to improve it a bit with a low effort (we don't\r\nwant to spend to much time on it).\r\n\r\nCurrent URL drilldown authoring experience is terrible mainly because\r\nthere is no proper validation while creating the drilldown as we don't\r\nhave the needed runtime context. In the initial version we had a preview\r\nbut it was very limited and used \"dummy\" context and in some cases got\r\nin the way by blocking the \"save\" button for URLs that would have been\r\nvalid in runtime. We simply removed the preview and validaiton on some\r\npoint later, so you can create an URL drilldown only by trial and error.\r\nThis is still the case in this PR, but it slightly improve the\r\nexperience:\r\n\r\nFirstly, **ONLY IN EDIT MODE** instead of hidding \"invalid\" drilldowns,\r\nwe're showing them now with an error. This helps to find broken\r\ndrilldowns and address issues. fixes\r\nhttps://github.com//issues/163641\r\n\r\n\r\n\r\nThis is far from ideal, but this is better from what we have now. \r\nAs for the error UI I wanted to use EuiIconTip, but it doesn't work well\r\nwhen inside that context menu because tooltip is shown below the menu. I\r\ndidn't want to change zIndex as it might cause regressions in other\r\nplaces, so I went for this inline error truncated after 3 lines and the\r\nwhole error is shown in native browser tooltip when hovered. The error\r\nis also printed in console\r\n\r\nIn addition to that I've slightly improved the form editor experience \r\n- Show simple non-blocking validation error (if URL is missing or the\r\npattern doesn't look like an URL)\r\n- Add a help text about how to test and properly validate \r\n\r\n<img width=\"576\" alt=\"Screenshot 2024-10-24 at 15 35 01\"\r\nsrc=\"https://github.com/user-attachments/assets/248b5c03-d445-4b30-97a2-a0a9d6a055a4\">\r\n<img width=\"547\" alt=\"Screenshot 2024-10-24 at 15 35 08\"\r\nsrc=\"https://github.com/user-attachments/assets/c265ee1c-94ec-4238-85fb-fc90f069f3b4\">\r\n\r\n\r\nThis is again, not ideal, but slighltly better then before","sha":"014b956002fab12064e2ac729c09b1713ef8deac","branchLabelMapping":{"^v9.0.0$":"main","^v8.17.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:fix","Feature:Drilldowns","v9.0.0","Team:SharedUX","backport:prev-minor"],"title":"Improve URL drilldown authoring experience ","number":197454,"url":"https://github.com/elastic/kibana/pull/197454","mergeCommit":{"message":"Improve URL drilldown authoring experience (#197454)\n\n## Summary\r\n\r\nclose https://github.com/elastic/kibana/issues/163642\r\nclose https://github.com/elastic/kibana/issues/163641\r\n\r\nAs part of fix-it-week I picked up some of existing URL drilldown\r\nauthoring issues hoping to improve it a bit with a low effort (we don't\r\nwant to spend to much time on it).\r\n\r\nCurrent URL drilldown authoring experience is terrible mainly because\r\nthere is no proper validation while creating the drilldown as we don't\r\nhave the needed runtime context. In the initial version we had a preview\r\nbut it was very limited and used \"dummy\" context and in some cases got\r\nin the way by blocking the \"save\" button for URLs that would have been\r\nvalid in runtime. We simply removed the preview and validaiton on some\r\npoint later, so you can create an URL drilldown only by trial and error.\r\nThis is still the case in this PR, but it slightly improve the\r\nexperience:\r\n\r\nFirstly, **ONLY IN EDIT MODE** instead of hidding \"invalid\" drilldowns,\r\nwe're showing them now with an error. This helps to find broken\r\ndrilldowns and address issues. fixes\r\nhttps://github.com//issues/163641\r\n\r\n\r\n\r\nThis is far from ideal, but this is better from what we have now. \r\nAs for the error UI I wanted to use EuiIconTip, but it doesn't work well\r\nwhen inside that context menu because tooltip is shown below the menu. I\r\ndidn't want to change zIndex as it might cause regressions in other\r\nplaces, so I went for this inline error truncated after 3 lines and the\r\nwhole error is shown in native browser tooltip when hovered. The error\r\nis also printed in console\r\n\r\nIn addition to that I've slightly improved the form editor experience \r\n- Show simple non-blocking validation error (if URL is missing or the\r\npattern doesn't look like an URL)\r\n- Add a help text about how to test and properly validate \r\n\r\n<img width=\"576\" alt=\"Screenshot 2024-10-24 at 15 35 01\"\r\nsrc=\"https://github.com/user-attachments/assets/248b5c03-d445-4b30-97a2-a0a9d6a055a4\">\r\n<img width=\"547\" alt=\"Screenshot 2024-10-24 at 15 35 08\"\r\nsrc=\"https://github.com/user-attachments/assets/c265ee1c-94ec-4238-85fb-fc90f069f3b4\">\r\n\r\n\r\nThis is again, not ideal, but slighltly better then before","sha":"014b956002fab12064e2ac729c09b1713ef8deac"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/197454","number":197454,"mergeCommit":{"message":"Improve URL drilldown authoring experience (#197454)\n\n## Summary\r\n\r\nclose https://github.com/elastic/kibana/issues/163642\r\nclose https://github.com/elastic/kibana/issues/163641\r\n\r\nAs part of fix-it-week I picked up some of existing URL drilldown\r\nauthoring issues hoping to improve it a bit with a low effort (we don't\r\nwant to spend to much time on it).\r\n\r\nCurrent URL drilldown authoring experience is terrible mainly because\r\nthere is no proper validation while creating the drilldown as we don't\r\nhave the needed runtime context. In the initial version we had a preview\r\nbut it was very limited and used \"dummy\" context and in some cases got\r\nin the way by blocking the \"save\" button for URLs that would have been\r\nvalid in runtime. We simply removed the preview and validaiton on some\r\npoint later, so you can create an URL drilldown only by trial and error.\r\nThis is still the case in this PR, but it slightly improve the\r\nexperience:\r\n\r\nFirstly, **ONLY IN EDIT MODE** instead of hidding \"invalid\" drilldowns,\r\nwe're showing them now with an error. This helps to find broken\r\ndrilldowns and address issues. fixes\r\nhttps://github.com//issues/163641\r\n\r\n\r\n\r\nThis is far from ideal, but this is better from what we have now. \r\nAs for the error UI I wanted to use EuiIconTip, but it doesn't work well\r\nwhen inside that context menu because tooltip is shown below the menu. I\r\ndidn't want to change zIndex as it might cause regressions in other\r\nplaces, so I went for this inline error truncated after 3 lines and the\r\nwhole error is shown in native browser tooltip when hovered. The error\r\nis also printed in console\r\n\r\nIn addition to that I've slightly improved the form editor experience \r\n- Show simple non-blocking validation error (if URL is missing or the\r\npattern doesn't look like an URL)\r\n- Add a help text about how to test and properly validate \r\n\r\n<img width=\"576\" alt=\"Screenshot 2024-10-24 at 15 35 01\"\r\nsrc=\"https://github.com/user-attachments/assets/248b5c03-d445-4b30-97a2-a0a9d6a055a4\">\r\n<img width=\"547\" alt=\"Screenshot 2024-10-24 at 15 35 08\"\r\nsrc=\"https://github.com/user-attachments/assets/c265ee1c-94ec-4238-85fb-fc90f069f3b4\">\r\n\r\n\r\nThis is again, not ideal, but slighltly better then before","sha":"014b956002fab12064e2ac729c09b1713ef8deac"}}]}] BACKPORT--> Co-authored-by: Anton Dosov <[email protected]>
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
close #163642
close #163641
As part of fix-it-week I picked up some of existing URL drilldown authoring issues hoping to improve it a bit with a low effort (we don't want to spend to much time on it).
Current URL drilldown authoring experience is terrible mainly because there is no proper validation while creating the drilldown as we don't have the needed runtime context. In the initial version we had a preview but it was very limited and used "dummy" context and in some cases got in the way by blocking the "save" button for URLs that would have been valid in runtime. We simply removed the preview and validaiton on some point later, so you can create an URL drilldown only by trial and error. This is still the case in this PR, but it slightly improve the experience:
Firstly, ONLY IN EDIT MODE instead of hidding "invalid" drilldowns, we're showing them now with an error. This helps to find broken drilldowns and address issues. fixes #163641
This is far from ideal, but this is better from what we have now.
As for the error UI I wanted to use EuiIconTip, but it doesn't work well when inside that context menu because tooltip is shown below the menu. I didn't want to change zIndex as it might cause regressions in other places, so I went for this inline error truncated after 3 lines and the whole error is shown in native browser tooltip when hovered. The error is also printed in console
In addition to that I've slightly improved the form editor experience
This is again, not ideal, but slighltly better then before