Skip to content

[Canvas] Adds function reference docs generator#49402

Merged
cqliu1 merged 20 commits intoelastic:masterfrom
cqliu1:docs/function-ref-script
Aug 24, 2020
Merged

[Canvas] Adds function reference docs generator#49402
cqliu1 merged 20 commits intoelastic:masterfrom
cqliu1:docs/function-ref-script

Conversation

@cqliu1
Copy link
Contributor

@cqliu1 cqliu1 commented Oct 25, 2019

Summary

This adds a button to the global nav help menu in the Canvas app for developers to generate the function reference. This button only appears in development, so it's not a user-facing feature.

Screen Shot 2020-07-27 at 7 54 09 PM

Once you click the button, the entire markdown content of the function reference docs will be stored to your clipboard, and then you'll have to navigate to kibana/docs/canvas/canvas-function-reference.asciidoc and paste in the generated content.

Because all of our function definitions support i18n, we have to generate the docs while Kibana is running, so I couldn't create a nice script to generate the docs and write the changes to the asciidoc file directly. This is a clunky workflow, but since it's not user-facing, I think it works well enough to get the job done. I'm open to suggestions for a better UX.

Checklist

Use strikethroughs to remove checklist items you don't feel are applicable to this PR.

For maintainers

@cqliu1 cqliu1 force-pushed the docs/function-ref-script branch 2 times, most recently from 0609fc7 to c256ab7 Compare October 25, 2019 20:39
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@cqliu1 cqliu1 force-pushed the docs/function-ref-script branch from c256ab7 to 41d8e23 Compare October 26, 2019 00:02
@cqliu1 cqliu1 changed the title [Canvas] Adds script for generating function reference docs [Canvas] Adds function reference docs generator Oct 26, 2019
@cqliu1 cqliu1 force-pushed the docs/function-ref-script branch 5 times, most recently from 1e54f15 to 59e3215 Compare October 26, 2019 01:29
@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@cqliu1 cqliu1 force-pushed the docs/function-ref-script branch from fc1dcd5 to b58b9eb Compare October 29, 2019 19:26
@elasticmachine
Copy link
Contributor

💔 Build Failed

@cqliu1 cqliu1 force-pushed the docs/function-ref-script branch from 1ddbd5a to 9add681 Compare October 29, 2019 21:32
@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@cqliu1 cqliu1 force-pushed the docs/function-ref-script branch from e5f6d12 to b98f061 Compare October 31, 2019 16:38
@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@cqliu1 cqliu1 force-pushed the docs/function-ref-script branch from 042377d to f393201 Compare November 11, 2019 16:18
@elasticmachine
Copy link
Contributor

💔 Build Failed

Copy link
Contributor

@lukeelmers lukeelmers left a comment

Choose a reason for hiding this comment

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

@cqliu1 I know this is just in draft state, but I'll leave a few notes as a follow up from our offline conversations last week. If you are looking for a final review once this is out of draft, feel free to request me and I'd be happy to run it locally and test it out.

Overall I don't think it matters exactly how this is implemented right now, since the only use for these is Canvas autocomplete. If we were to officially expose this example data as part of the larger public expressions service, I'd want to think more about whether the function definition is the right place to introduce arbitrary metadata like this.

Functionally, this metadata exists solely for the purposes of displaying content in the UI... it doesn't have anything to do with how the function is handled by the interpreter. So on the one hand, I like the idea of trying to separate these concerns. But on the other hand, I don't know that a separate "examples registry" or whatever is the most elegant way to go about this longer term; this is something we'd need to explore further if we see more and more use cases like this popping up.

For the time being, I have no major concerns on my end as long as we are clearly marking this portion of the interface as "internal" and for use in Canvas only... that way we have flexibility to change things up later if needed.

cc @ppisljar I think this is in line with what we discussed too, feel free to add anything I've missed.

@cqliu1 cqliu1 force-pushed the docs/function-ref-script branch from f393201 to 3261b2c Compare November 14, 2019 19:37
@elasticmachine
Copy link
Contributor

💔 Build Failed

@cqliu1 cqliu1 force-pushed the docs/function-ref-script branch from ffe344e to c70fe29 Compare August 4, 2020 16:45
Copy link
Contributor

@poffdeluxe poffdeluxe left a comment

Choose a reason for hiding this comment

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

overall stuff looks good -- I did find an issue with that str.contains stuff though and I have a few nits. Also, a functional test would be great if possible

${createFunctionDocs(functionDictionary)}`;
};

const createAlphabetLinks = (functionDictionary: FunctionDictionary) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: might make sense to put all these helper funcs into a lib or something somewhere. Feels a bit big for a component file

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I extract all the doc related functions into a separate file in the same component folder.

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Build metrics

async chunks size

id value diff baseline
canvas 1.4MB -1.2KB 1.4MB

page load bundle size

id value diff baseline
canvas 1.3MB +527.0B 1.3MB

History

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

@cqliu1 cqliu1 merged commit 3256992 into elastic:master Aug 24, 2020
@cqliu1 cqliu1 deleted the docs/function-ref-script branch August 24, 2020 21:28
@cqliu1 cqliu1 removed the v7.9.0 label Aug 24, 2020
cqliu1 added a commit that referenced this pull request Aug 25, 2020
Co-authored-by: Corey Robertson <corey.robertson@elastic.co>
# Conflicts:
#	x-pack/plugins/canvas/i18n/functions/dict/map_column.ts
@cqliu1 cqliu1 restored the docs/function-ref-script branch June 8, 2022 14:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

chore loe:x-large Extra Large Level of Effort release_note:skip Skip the PR/issue when compiling release notes review Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas t// v7.10.0 v8.0.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants