Skip to content
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

[ML] Add doc link in help menu on more ML pages #86500

Merged
merged 9 commits into from
Jan 4, 2021
Merged

Conversation

lcawl
Copy link
Contributor

@lcawl lcawl commented Dec 18, 2020

Summary

Related to #85088

This PR adds documentation links in the help menu on all of the pages in the Machine Learning app that contain a NavigationMenu. For example:

image

@lcawl lcawl added :ml v8.0.0 release_note:skip Skip the PR/issue when compiling release notes v7.12.0 labels Dec 18, 2020
@lcawl lcawl requested a review from a team as a code owner December 18, 2020 18:35
@elasticmachine
Copy link
Contributor

Pinging @elastic/ml-ui (:ml)

@lcawl
Copy link
Contributor Author

lcawl commented Dec 21, 2020

@elasticmachine merge upstream

@@ -130,6 +135,7 @@ export const Page: FC = () => {
</EuiPageContent>
</EuiPageBody>
</EuiPage>
<HelpMenu docLink={helpLink} />
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it may be better if we add the HelpMenu link into all the ML pages in one single PR. Otherwise if you switch to a page where it isn't explicitly set like this, it will remain pointing to the last configured link. For example, here on the Data Visualizer page the link will open up https://www.elastic.co/guide/en/machine-learning/master/ml-dfanalytics.html as the Data Visualizer page doesn't contain its own HelpMenu tag.

https://www.elastic.co/guide/en/machine-learning/master/ml-dfanalytics.html

From a quick look, I think you need to set it on all the pages which contain NavigationMenu, which would be:

./access_denied/page.tsx
./datavisualizer/datavisualizer_selector.tsx
./datavisualizer/file_based/file_datavisualizer.tsx
./datavisualizer/index_based/page.tsx
./data_frame_analytics/pages/analytics_exploration/page.tsx
./data_frame_analytics/pages/analytics_management/page.tsx
./explorer/explorer.js
./jobs/jobs_list/jobs.tsx
./overview/overview_page.tsx
./settings/calendars/edit/new_calendar.js
./settings/calendars/list/calendars_list.js
./settings/filter_lists/edit/edit_filter_list.js
./settings/filter_lists/list/filter_lists.js
./settings/settings.tsx
./timeseriesexplorer/timeseriesexplorer_page.tsx

@lcawl lcawl added the WIP Work in progress label Dec 21, 2020
@lcawl lcawl changed the title [ML] Add doc link in help menu on data frame analytics page [ML] Add doc link in help menu on more ML pages Dec 22, 2020
@lcawl lcawl removed the WIP Work in progress label Dec 22, 2020
@lcawl
Copy link
Contributor Author

lcawl commented Dec 30, 2020

@elasticmachine merge upstream

@lcawl lcawl requested review from alvarezmelissa87 and a team December 31, 2020 02:58
@lcawl
Copy link
Contributor Author

lcawl commented Jan 4, 2021

@elasticmachine merge upstream

Copy link
Contributor

@peteharverson peteharverson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Latest edits LGTM

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
ml 7.0MB 7.0MB +3.3KB

Distributable file count

id before after diff
default 47266 48026 +760

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@lcawl lcawl merged commit 96e9fdd into elastic:master Jan 4, 2021
@lcawl lcawl deleted the help-dfa branch January 4, 2021 20:32
lcawl added a commit to lcawl/kibana that referenced this pull request Jan 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:ml release_note:skip Skip the PR/issue when compiling release notes v7.12.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants