-
Notifications
You must be signed in to change notification settings - Fork 8.1k
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
[Vislib XY axis] Adds a deprecation notice in the UI and a docs section #105055
[Vislib XY axis] Adds a deprecation notice in the UI and a docs section #105055
Conversation
import { | ||
SavedVisInstance, | ||
VisualizeAppState, | ||
VisualizeAppStateContainer, | ||
VisualizeEditorVisInstance, | ||
} from '../types'; | ||
import { getUISettings } from '../../services'; | ||
|
||
const LEGACY_CHARTS_LIBRARY = 'visualization:visualize:legacyChartsLibrary'; |
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.
I don't import it from xy plugin because it creates a cyclic dependency. As this code and setting is going to removed in the next minor, I think it is ok.
@elasticmachine merge upstream |
@elasticmachine merge upstream |
Pinging @elastic/kibana-app (Team:KibanaApp) |
@gchaps @KOTungseth I need your help on this. Moreover, I added some documentation explaining the advantages of the new implementation. I would love your feedback on the UI text and the documentation added. |
src/plugins/visualize/public/application/components/deprecation_vis_warning.tsx
Outdated
Show resolved
Hide resolved
src/plugins/visualize/public/application/components/visualize_editor_common.tsx
Show resolved
Hide resolved
@wylieconlon @gchaps User has the permissions. I have added one link to the advanced settings switch and one to our documentation. User without permissions. Only link to the documentation and a message to contact the administrator. |
Co-authored-by: Kaarina Tungseth <kaarina.tungseth@elastic.co>
@elasticmachine merge upstream |
@gchaps and @KOTungseth thank you! It is much better now! I removed all the changes I made on the aggregation-based section and moved them as Gail suggested on the advanced settings page. Here are the screenshots of the UI notification: |
@elasticmachine merge upstream |
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.
Haven't run the code, but changes LGTM.
src/plugins/visualize/public/application/components/deprecation_vis_warning.tsx
Outdated
Show resolved
Hide resolved
Re: the warning, since Advanced Settings is a feature, it should use title case. |
💚 Build Succeeded
Metrics [docs]Module Count
Async chunks
History
To update your PR or re-run it, just comment with: |
…on (elastic#105055) * [Vislib XY axis] Adds a deprecation notice in the UI and a docs section * Remove cyclic dependency * Fix link * Add functional test * fix functional tests * Apply PR comments * Update docs/user/dashboard/aggregation-based.asciidoc Co-authored-by: Kaarina Tungseth <kaarina.tungseth@elastic.co> * Apply PR changes * minor * Change the implementation * Use title calse in Advanced Settings Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Kaarina Tungseth <kaarina.tungseth@elastic.co>
…on (#105055) (#106221) * [Vislib XY axis] Adds a deprecation notice in the UI and a docs section * Remove cyclic dependency * Fix link * Add functional test * fix functional tests * Apply PR comments * Update docs/user/dashboard/aggregation-based.asciidoc Co-authored-by: Kaarina Tungseth <kaarina.tungseth@elastic.co> * Apply PR changes * minor * Change the implementation * Use title calse in Advanced Settings Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Kaarina Tungseth <kaarina.tungseth@elastic.co> Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Kaarina Tungseth <kaarina.tungseth@elastic.co>
Summary
It adds a notification on the vislib XY axis charts. The notification is only visible on the editor and not on the dashboard.
Part of #103209
Documentation preview https://kibana_105055.docs-preview.app.elstc.co/guide/en/kibana/master/add-aggregation-based-visualization-panels.html#new-charts-library
Checklist
Delete any items that are not applicable to this PR.