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

[APM] Move APM feedback link to global Help popover #30918

Merged
merged 4 commits into from
Feb 20, 2019

Conversation

ogupte
Copy link
Contributor

@ogupte ogupte commented Feb 13, 2019

Closes #29687 by removing the old APM feedback link from the datepicker section and adding the link as custom content in the global help popover.

screen shot 2019-02-12 at 10 30 36 pm

@ogupte ogupte self-assigned this Feb 13, 2019
@ogupte ogupte requested a review from a team as a code owner February 13, 2019 06:36
Copy link
Contributor

@formgeist formgeist left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@jasonrhodes jasonrhodes left a comment

Choose a reason for hiding this comment

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

LGTM!

@ogupte
Copy link
Contributor Author

ogupte commented Feb 15, 2019

retest

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@@ -28,6 +30,25 @@ import { history } from './components/shared/Links/url_helpers';

import { I18nContext } from 'ui/i18n';

// render APM feedback link in global help menu
chrome.helpExtension.set(domElement => {
Copy link
Member

Choose a reason for hiding this comment

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

Is this the only API for setting the help menu? I was expecting it to be more strictly defined - and if anything that it would assume React. I guess it's a good thing that it's framework agnostic - just not how I expected it to be...

Copy link
Contributor

@formgeist formgeist left a comment

Choose a reason for hiding this comment

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

@ogupte Sorrry to spring this on you so late in the implementation, but we're aiming at aligning this with the other Observability UI's as per #29687. I've added two tasks, one renaming the link label you added and another adding one more link to the Migration Assistant.

Thanks 👍

rel="noopener"
>
{i18n.translate('xpack.apm.feedbackMenu.provideFeedbackTitle', {
defaultMessage: 'Give APM Feedback'
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we change this to "Provide feedback for APM"?

defaultMessage: 'Give APM Feedback'
})}
</EuiLink>,
domElement
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we add another link to the Kibana Migration Assistant? Link is /app/kibana#/management/elasticsearch/upgrade_assistant and happens within Kibana so no target and no rel props necessary, afaik?

@formgeist
Copy link
Contributor

@ogupte Looks good to me, thanks 👍

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@ogupte
Copy link
Contributor Author

ogupte commented Feb 20, 2019

retest

@elasticmachine
Copy link
Contributor

💔 Build Failed

@ogupte
Copy link
Contributor Author

ogupte commented Feb 20, 2019

retest

@elasticmachine
Copy link
Contributor

💔 Build Failed

@ogupte ogupte force-pushed the apm-29687-feedback-link-global-help branch from fbaa427 to ba43742 Compare February 20, 2019 20:58
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@ogupte ogupte merged commit 5d22c25 into elastic:master Feb 20, 2019
ogupte added a commit to ogupte/kibana that referenced this pull request Feb 20, 2019
* [APM] Closes elastic#29687 by removing the old APM feedback link from the datepicker
section and adding the link as custom content in the global help popover.

* remove noreferrer property for the feedback link

* [APM] updated feedback link text and added help link to migration assistant

* [APM] remove unused translations related to the old feedback link
ogupte added a commit to ogupte/kibana that referenced this pull request Feb 20, 2019
* [APM] Closes elastic#29687 by removing the old APM feedback link from the datepicker
section and adding the link as custom content in the global help popover.

* remove noreferrer property for the feedback link

* [APM] updated feedback link text and added help link to migration assistant

* [APM] remove unused translations related to the old feedback link
ogupte added a commit that referenced this pull request Feb 20, 2019
* [APM] Closes #29687 by removing the old APM feedback link from the datepicker
section and adding the link as custom content in the global help popover.

* remove noreferrer property for the feedback link

* [APM] updated feedback link text and added help link to migration assistant

* [APM] remove unused translations related to the old feedback link
ogupte added a commit that referenced this pull request Feb 20, 2019
* [APM] Closes #29687 by removing the old APM feedback link from the datepicker
section and adding the link as custom content in the global help popover.

* remove noreferrer property for the feedback link

* [APM] updated feedback link text and added help link to migration assistant

* [APM] remove unused translations related to the old feedback link
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants