Migrate reporting top nav to sharing context menu #22596
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
💔 Build Failed |
💔 Build Failed |
|
@elastic/kibana-design Where should the feedback go when the user clicks the generate button? Right now a toast notification is displayed. @cjcenizal suggested disabling the button and displaying the message in the context menu. I like that idea since it puts the feedback closer to where the action takes place. What do you think?
|
|
I just thought of one problem with my idea of disabling the button... if the user has auto-refresh enabled and wants to create a series of PDFs as the data changes, then disabling it on click will be frustrating. |
💔 Build Failed |
💔 Build Failed |
💔 Build Failed |
💔 Build Failed |
💔 Build Failed |
|
bootstrap error |
|
jenkins, test this |
💔 Build Failed |
|
jenkins, test this |
💔 Build Failed |
💔 Build Failed |
|
jenkins, test this |
💔 Build Failed |
💔 Build Failed |
|
flaky x-pack security test jenkins, test this |
💚 Build Succeeded |
💔 Build Failed |
|
Same xpack rbac test failure. @elastic/kibana-security Is this a known flaky test? jenkins, test this |
|
@Rasroh is this the flaky test you wanted to debug with me? |
💚 Build Succeeded |
stacey-gammon
approved these changes
Sep 11, 2018
src/ui/public/share/share_action.ts
Outdated
| readonly id: string; | ||
|
|
||
| getShareActions: ( | ||
| { objectType, objectId, getUnhashableStates, sharingData, isDirty, onClose }: ShareActionProps |
There was a problem hiding this comment.
Interesting way to do this. Since it's just defining the interface and ShareAction, I think you could get away with:
getShareActions: (config: ShareActionProps) => ShareAction[]
There was a problem hiding this comment.
not sure if you are looking at an old commits, but the destructuring was removed in this commit
There was a problem hiding this comment.
Sorry, that was an old unpublished comment that got published by accident with the latest :)
| const query = { | ||
| jobParams: rison.encode(jobParams), | ||
| }; | ||
| return kfetch({ method: 'POST', pathname: `${API_BASE_URL}/${exportType}`, query }).then( |
There was a problem hiding this comment.
I think if you make this an async function and use await the returned promise is implicit. Should still work with the catch. E.g.
const response = await kfetch({ method: 'POST', pathname: `${API_BASE_URL}/${exportType}`, query });
jobCompletionNotifications.add(response.job.id);
I don't think you need to return the response, just the promise.
| * you may not use this file except in compliance with the Elastic License. | ||
| */ | ||
|
|
||
| // TODO: Remove once typescript definitions are in EUI |
There was a problem hiding this comment.
I think you can remove this comment, there is no declaration here, so I think these defintions are typed in eui
💚 Build Succeeded |
|
cc @w33ble Not sure how this is going to impact canvas/reports. |
nreese
added
the
release_note:plugin_api_changes
💔 Build Failed |
sorenlouv
reviewed
Sep 21, 2018
| @@ -85,6 +85,7 @@ | |||
| "@kbn/ui-framework": "link:../packages/kbn-ui-framework", | |||
| "@samverschueren/stream-to-observable": "^0.3.0", | |||
| "@slack/client": "^4.2.2", | |||
| "@types/moment-timezone": "^0.5.8", | |||
There was a problem hiding this comment.
@nreese Do you mind if I move this to devDependencies?
There was a problem hiding this comment.
Not at all. Thanks @sqren. Putting it in dependencies was a mistake
There was a problem hiding this comment.
No worries. I just noticed as I was adding some types.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Dev Docs
exportTypesRegistryremovedRemoved
exportTypesRegistryfrom the client-side.exportTypesRegistryhad been used for registering Reporting export types for the UI.exportTypesRegistrystill exists for registering export types with the server.