[Security Solution] Display readonly placeholders when field value is empty #203826
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
nikitaindik
added
release_note:skip
69 tasks
nikitaindik
force-pushed
the
placeholders-for-empty-fields
branch
2 times, most recently
from
d0fc648 to
3698520
Compare
|
Pinging @elastic/security-detections-response (Team:Detections and Resp) |
|
Pinging @elastic/security-solution (Team: SecuritySolution) |
|
Pinging @elastic/security-detection-rule-management (Team:Detection Rule Management) |
maximpn
reviewed
Dec 13, 2024
There was a problem hiding this comment.
@nikitaindik I looked at the diff and tested the PR locally. UI looks much nicer with empty values placeholders 👍
I've noticed a problem with note and setup fields. These fields allow space characters. A bunch of spaces look like an empty value. Spaces don't provide any value and we should interpret that fields as empty when values contain only space characters. trim() should help with that.
I tested only UI. Rule upgrade is blocked by #203634. The following diffable rule fields were tested
Optional fields
- ✅
investigation_fields - ✅
rule_name_override - ✅
timestamp_override - ✅
timeline_template - ✅
building_block - ✅
alert_suppression - ✅
threat_indicator_path - ✅
data_source
String fields allowing empty strings
⚠️ notespace characters produce visually appearing empty result⚠️ setupspace characters produce visually appearing empty result
Array fields allowing empty arrays
- ✅
tags - ✅
references - ✅
false_positives - ✅
threat - ✅
related_integrations - ✅
required_fields - ✅
severity_mapping - ✅
risk_score_mapping
...hree_way_diff/final_readonly/empty_field_value_placeholder/empty_field_value_placeholder.tsx
Show resolved
Hide resolved
...rule_management/components/rule_details/three_way_diff/final_readonly/fields/setup/setup.tsx
Outdated
Show resolved
Hide resolved
.../rule_details/three_way_diff/final_readonly/fields/timestamp_override/timestamp_override.tsx
Outdated
Show resolved
Hide resolved
...ution/public/detection_engine/rule_management/components/rule_details/rule_about_section.tsx
Outdated
Show resolved
Hide resolved
| export const BUILDING_BLOCK_DISABLED_FIELD_DESCRIPTION = i18n.translate( | ||
| 'xpack.securitySolution.detectionEngine.ruleDetails.buildingBlockDisabledFieldDescription', | ||
| { | ||
| defaultMessage: 'Will not mark alerts as "building block" alerts', |
There was a problem hiding this comment.
Ideally we should have a sentence without negation but we don't have a term normal alerts. It's better to consult with docs folks regarding this text.
There was a problem hiding this comment.
Discussed this with Georgii. Decided to make add it as an item into the docs ticket.
|
@maximpn Thx for the review. I've addressed the feedback. |
maximpn
approved these changes
Dec 13, 2024
There was a problem hiding this comment.
@nikitaindik Thanks for addressing my comments 👍
I see the build failed due to a single occurrence of isBuildingBlockEnabled not replaced with type.
| @@ -294,7 +303,7 @@ const prepareAboutSectionListItems = ( | |||
| title: ( | |||
| <span data-test-subj="buildingBlockPropertyTitle">{i18n.BUILDING_BLOCK_FIELD_LABEL}</span> | |||
| ), | |||
| description: <BuildingBlock />, | |||
| description: <BuildingBlock isBuildingBlockEnabled={true} />, | |||
There was a problem hiding this comment.
Suggested change
| description: <BuildingBlock isBuildingBlockEnabled={true} />, | |
| description: <BuildingBlock type="default" />, |
|
Starting backport for target branches: 8.x https://github.com/elastic/kibana/actions/runs/12322291412 |
💚 Build Succeeded
Metrics [docs]Module Count
Async chunks
History
cc @nikitaindik |
kibanamachine
pushed a commit
to kibanamachine/kibana
that referenced
this pull request
Dec 13, 2024
… empty (elastic#203826) **Partially addresses: elastic#171520 ## Summary This PR updates readonly components of the Rule Upgrade flyout to display placeholders in cases when a field value is empty. ## Changes - Added placeholders to readonly components in the Rule Upgrade flyout - Simplified Storybook stories to make them more readable and less dependent on flyout context - Added stories that showcase empty states. You can run Storybook with `yarn storybook security_solution`. ## Screenshots **Before / After** <img width="2560" alt="Schermafbeelding 2024-12-12 om 00 05 56" src="https://github.com/user-attachments/assets/e85a4514-4861-4be7-b0d2-534f7ad2d8cf" /> Work started on 11-Dec-2024 (cherry picked from commit 27fe7e4)
💚 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
Dec 13, 2024
…lue is empty (#203826) (#204279) # Backport This will backport the following commits from `main` to `8.x`: - [[Security Solution] Display readonly placeholders when field value is empty (#203826)](#203826) <!--- Backport version: 9.4.3 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT [{"author":{"name":"Nikita Indik","email":"nikita.indik@elastic.co"},"sourceCommit":{"committedDate":"2024-12-13T20:06:40Z","message":"[Security Solution] Display readonly placeholders when field value is empty (#203826)\n\n**Partially addresses: https://github.com/elastic/kibana/issues/171520**\r\n\r\n## Summary\r\nThis PR updates readonly components of the Rule Upgrade flyout to\r\ndisplay placeholders in cases when a field value is empty.\r\n\r\n## Changes\r\n - Added placeholders to readonly components in the Rule Upgrade flyout\r\n- Simplified Storybook stories to make them more readable and less\r\ndependent on flyout context\r\n- Added stories that showcase empty states. You can run Storybook with\r\n`yarn storybook security_solution`.\r\n\r\n## Screenshots\r\n**Before / After**\r\n<img width=\"2560\" alt=\"Schermafbeelding 2024-12-12 om 00 05 56\"\r\nsrc=\"https://github.com/user-attachments/assets/e85a4514-4861-4be7-b0d2-534f7ad2d8cf\"\r\n/>\r\n\r\nWork started on 11-Dec-2024","sha":"27fe7e49e53807597ab47aca7ecfe40ac8d6e5fd","branchLabelMapping":{"^v9.0.0$":"main","^v8.18.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","v9.0.0","Team:Detections and Resp","Team: SecuritySolution","Team:Detection Rule Management","Feature:Prebuilt Detection Rules","backport:version","v8.18.0"],"title":"[Security Solution] Display readonly placeholders when field value is empty","number":203826,"url":"https://github.com/elastic/kibana/pull/203826","mergeCommit":{"message":"[Security Solution] Display readonly placeholders when field value is empty (#203826)\n\n**Partially addresses: https://github.com/elastic/kibana/issues/171520**\r\n\r\n## Summary\r\nThis PR updates readonly components of the Rule Upgrade flyout to\r\ndisplay placeholders in cases when a field value is empty.\r\n\r\n## Changes\r\n - Added placeholders to readonly components in the Rule Upgrade flyout\r\n- Simplified Storybook stories to make them more readable and less\r\ndependent on flyout context\r\n- Added stories that showcase empty states. You can run Storybook with\r\n`yarn storybook security_solution`.\r\n\r\n## Screenshots\r\n**Before / After**\r\n<img width=\"2560\" alt=\"Schermafbeelding 2024-12-12 om 00 05 56\"\r\nsrc=\"https://github.com/user-attachments/assets/e85a4514-4861-4be7-b0d2-534f7ad2d8cf\"\r\n/>\r\n\r\nWork started on 11-Dec-2024","sha":"27fe7e49e53807597ab47aca7ecfe40ac8d6e5fd"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/203826","number":203826,"mergeCommit":{"message":"[Security Solution] Display readonly placeholders when field value is empty (#203826)\n\n**Partially addresses: https://github.com/elastic/kibana/issues/171520**\r\n\r\n## Summary\r\nThis PR updates readonly components of the Rule Upgrade flyout to\r\ndisplay placeholders in cases when a field value is empty.\r\n\r\n## Changes\r\n - Added placeholders to readonly components in the Rule Upgrade flyout\r\n- Simplified Storybook stories to make them more readable and less\r\ndependent on flyout context\r\n- Added stories that showcase empty states. You can run Storybook with\r\n`yarn storybook security_solution`.\r\n\r\n## Screenshots\r\n**Before / After**\r\n<img width=\"2560\" alt=\"Schermafbeelding 2024-12-12 om 00 05 56\"\r\nsrc=\"https://github.com/user-attachments/assets/e85a4514-4861-4be7-b0d2-534f7ad2d8cf\"\r\n/>\r\n\r\nWork started on 11-Dec-2024","sha":"27fe7e49e53807597ab47aca7ecfe40ac8d6e5fd"}},{"branch":"8.x","label":"v8.18.0","branchLabelMappingKey":"^v8.18.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}] BACKPORT--> Co-authored-by: Nikita Indik <nikita.indik@elastic.co>
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.
Partially addresses: #171520
Summary
This PR updates readonly components of the Rule Upgrade flyout to display placeholders in cases when a field value is empty.
Changes
yarn storybook security_solution.Screenshots
Before / After
Work started on 11-Dec-2024