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.

Sign up for GitHub

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

Jump to bottom

[Lens] Use the language-documentation package for formula #143649

Merged
merged 9 commits into from
Oct 21, 2022
Merged

[Lens] Use the language-documentation package for formula #143649

merged 9 commits into from
Oct 21, 2022

Conversation

stratoula
Copy link
Contributor

@stratoula stratoula commented Oct 19, 2022 •
edited
Loading

Summary

Closes #137933

On a previous PR we created a new package which exports the stateless components for displaying documentation in a component.

This PR uses this package for Lens formula and also does some cleanup on unused css etc.

@stratoula
Copy link
Contributor Author

@elasticmachine run elasticsearch-ci/docs

@stratoula stratoula changed the title [Lens] Used documentation package for formula [Lens] Re use the language-documentation package for formula Oct 19, 2022
@stratoula stratoula changed the title [Lens] Re use the language-documentation package for formula [Lens] Use the language-documentation package for formula Oct 19, 2022
@stratoula
Copy link
Contributor Author

@elasticmachine run elasticsearch-ci/docs

@stratoula stratoula added technical debt Improvement of the software architecture and operational architecture release_note:skip Skip the PR/issue when compiling release notes Feature:Lens backport:skip This commit does not require backporting v8.6.0 labels Oct 20, 2022
@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
lens 935 943 +8

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id before after diff
@kbn/language-documentation-popover 4 5 +1

Async chunks

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

id before after diff
lens 1.3MB 1.3MB +1.6KB
unifiedSearch 262.1KB 260.4KB -1.7KB
total -46.0B
Unknown metric groups

API count

id before after diff
@kbn/language-documentation-popover 5 7 +2

History

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

@stratoula stratoula marked this pull request as ready for review October 20, 2022 08:09
@stratoula stratoula requested review from a team as code owners October 20, 2022 08:09
@stratoula stratoula added the Team:Visualizations Visualization editors, elastic-charts and infrastructure label Oct 20, 2022
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-vis-editors @elastic/kibana-vis-editors-external (Team:VisEditors)

dej611
dej611 approved these changes Oct 20, 2022
View reviewed changes
Copy link
Contributor

@dej611 dej611 left a comment

Choose a reason for hiding this comment

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

Tested locally 👍

MichaelMarcialis
MichaelMarcialis approved these changes Oct 20, 2022
View reviewed changes
Copy link
Contributor

@MichaelMarcialis MichaelMarcialis left a comment

Choose a reason for hiding this comment

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

Hey, @stratoula! Left one small comment about a possibly unnecessary style, but otherwise this looks good to me. Approving now.

packages/kbn-language-documentation-popover/src/components/documentation.scss
Comment on lines +5 to +7
.documentation__docsHeader {
margin: 0;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this style necessary? In testing locally, it didn't seem so, but I'm not sure if there's something I may have missed. If not needed, I imagine we can remove.

Suggested change
.documentation__docsHeader {
margin: 0;
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@MichaelMarcialis it is necessary for lens formula on fullscrren mode
This is how it looks without it
image

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh weird. I wasn't seeing it when I removed the style, but I probably didn't save the change or something silly like that. Thanks!

@stratoula stratoula merged commit 4349ea7 into elastic:main Oct 21, 2022
jloleysens added a commit to jloleysens/kibana that referenced this pull request Oct 24, 2022
@jloleysens
Merge branch 'main' into files-upload-design-feedback
da22f2f 
* main: (57 commits)
  [Files] Filepicker (elastic#143111)
  [Infrastructure UI] Replace Lens table with EUI table and own api (elastic#142871)
  [api-docs] Daily api_docs build (elastic#143829)
  [api-docs] Daily api_docs build (elastic#143825)
  [api-docs] Daily api_docs build (elastic#143823)
  [Security Solution] Restructuring folders of Detection Engine + refactoring Rule Management (elastic#142950)
  [Dev tools] Fix performance issue with autocomplete suggestions (elastic#143428)
  [Security Solution] Disable ML rule's edit button link under basic license (elastic#143260)
  [Lens]  Use the language-documentation package for formula (elastic#143649)
  [api-docs] Daily api_docs build (elastic#143811)
  [Security Solution] Fix missing title on inspect pop-up (elastic#143601)
  fix incorrect filters being passed to events table causing duplicate entries in our inpsect tool request tab (elastic#143239)
  [Security Solution][Endpoint] `get-file` response action kibana download file API (elastic#143708)
  Rely on refresh context to update stats independently of overview cards. (elastic#143308)
  [RAM] Rule event log - Fix incorrect results when filtering by message and outcome simultaneously (elastic#143119)
  [ML] Display link to create data view from error cases in data frame analytics results pages (elastic#143596)
  Update links in README :) (elastic#143675)
  Add more tests for ml_inference_logic (elastic#143764)
  skip failing test suite (elastic#143717)
  [DOCS] Add assignees to case APIs (elastic#143610)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MichaelMarcialis MichaelMarcialis MichaelMarcialis approved these changes

@jloleysens jloleysens jloleysens approved these changes

@dej611 dej611 dej611 approved these changes

Assignees
No one assigned
Labels
backport:skip This commit does not require backporting Feature:Lens release_note:skip Skip the PR/issue when compiling release notes Team:Visualizations Visualization editors, elastic-charts and infrastructure technical debt Improvement of the software architecture and operational architecture v8.6.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Lens] [Unified search] Move the formula and text-based languages documentation popover to a package
6 participants
@stratoula @kibana-ci @elasticmachine @dej611 @MichaelMarcialis @jloleysens