-
Notifications
You must be signed in to change notification settings - Fork 916
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
[OSCI] removed KUI usage in visualizations #5462
Conversation
Signed-off-by: Cailey Jen <caileyjen@gmail.com>
Signed-off-by: Cailey Jen <63242227+caileyjen@users.noreply.github.com>
className="kuiVerticalRhythm visDisabledLabVisualization__icon kuiIcon fa-flask" | ||
aria-hidden="true" | ||
<EuiEmptyPrompt | ||
iconType="database" |
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.
Seems icon "beaker" makes a bit more sense here? cuz it is experimental?
<Fragment> | ||
<p> | ||
Enable experimental visualizations within{' '} | ||
<EuiLink href="https://github.com/app/management/opensearch-dashboards/settings"> |
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.
Using a url directly seems not the best way to do this? According to #3208, doc links should be from DocLinksService
. Some places you could check:
- src/core/public/doc_links/doc_links_service.ts: could add a section
management: { advancedSettingsLink: "**"}
for Dashboards Management links - src/plugins/visualizations/public/services.ts: import
DocLinksStart
and addexport const [getDocLinks, setDocLinks] = createGetterSetter<DocLinksStart>('docLinks');
- src/plugins/visualizations/public/embeddable/disabled_lab_visualization.tsx: use
getDocLinks
to get the link and use it in href.
The links seems not correct to me as well. https://github.com/app/management/opensearch-dashboards/settings
is 404. I think this https://opensearch.org/docs/latest/dashboards/management/advanced-settings/
might be the one if you are trying to direct people to instructions on how to use advanced settings.
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.
Thank you! I wasn't completely sure about what I was supposed to link to. I'll make the appropriate changes.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #5462 +/- ##
=======================================
Coverage 66.82% 66.82%
=======================================
Files 3291 3291
Lines 63174 63174
Branches 10055 10055
=======================================
Hits 42213 42213
- Misses 18479 18551 +72
+ Partials 2482 2410 -72
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
@caileyjen Just checking in to see if you will be able to make some of Anan's suggestions. |
This PR completes opensearch-project#5462 Please refer to the above PR for more details Signed-off-by: Anan Zhuang <ananzh@amazon.com>
…PR (#6360) (#6407) This PR completes #5462 Please refer to the above PR for more details Signed-off-by: Anan Zhuang <ananzh@amazon.com> (cherry picked from commit 46b17e4) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Description
Removed KUI classes in the
DisabledLabVisualization
componentChanged placeholder text and icon should be with
OuiEmptyPrompt
Updated text to "[vispanel title] is an experimental visualization. Enable experimental visualizations within Advanced Settings."
Issues Resolved
#3386
Screenshot
Testing the changes
How to view the
DisabledVabVisualization
componentCheck List
yarn test:jest
yarn test:jest_integration