-
Notifications
You must be signed in to change notification settings - Fork 947
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.
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
[Discover] support description in data-structures #9296
base: main
Are you sure you want to change the base?
[Discover] support description in data-structures #9296
Conversation
Signed-off-by: Joshua Li <[email protected]>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #9296 +/- ##
==========================================
- Coverage 61.71% 61.70% -0.02%
==========================================
Files 3816 3816
Lines 91822 91831 +9
Branches 14542 14544 +2
==========================================
- Hits 56672 56668 -4
- Misses 31495 31507 +12
- Partials 3655 3656 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
{title} | ||
</EuiText> | ||
{item.description && ( | ||
<EuiText size="s" color="subdued" className="datasetTable__itemDescription"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should the title and description have the same size? can imagine if each structure has a description it can get lengthy
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
they are the same size?
OpenSearch-Dashboards/src/plugins/data/public/ui/dataset_selector/dataset_table.tsx
Lines 97 to 98 in 0404e58
<EuiText size="s" className="datasetTable__itemTitle"> | |
{title} |
i can use extra small if it's what you mean. currently there's no way for user to set description, it needs the dataset type config to define what description is
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
title and description are both small size text <EuiText size="s"
, maybe use xs
for description?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
</EuiText> | ||
{item.description && ( | ||
<EuiText size="s" color="subdued" className="datasetTable__itemDescription"> | ||
{item.description} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
May be add ellipsis and truncation to support longer descriptions? or it is that dataset type config would have this logic?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added #9296 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of adding custom css, could the utility css class from oui work? .oui-textTruncate
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Signed-off-by: Joshua Li <[email protected]>
188df54
to
f601b5d
Compare
Description
Sometimes a data structure contains other metadata, it would be helpful to display them as descriptions to the user. Examples can be user defined descriptions, tags, or associated resources.
This PR adds a optional description field for data structures, and it is displayed in the dataset table. Currently it does not add it to dataset selector but can be done in the future.
Issues Resolved
Screenshot
Testing the changes
unit tests added
Changelog
Check List
yarn test:jest
yarn test:jest_integration