[Painless Lab] Minor Fixes by jloleysens · Pull Request #58135 · elastic/kibana

jloleysens · 2020-02-20T15:38:11Z

Summary

Code restructure: we now have a custom hook for the call to submit code, making Main component a bit smaller. The hook contains all effectual code as before.
Introduced an on Context Change handler. Components further down the tree just call this component and indicate what piece of state has been updated.
Added the plugin ID (see URL issue below)
Re-introduced parsing of full error report (here 8652c02). The raw error report contains a lot of repeating information and can take a while to grok before spotting the source of the error
Removed the leading new line in the default smiley script
Fixed a lot of typings inaccuracies
There is still a browser error for the Output pane title.
Also removed the feature flag registration in advanced settings

Screenshots

URL `undefined`, now `painless_lab`

Error messages: curated vs raw

Error for `Output` pane title with icon

Moved the code execution hook to a custom hook outside of main, also chaining off promise to avoid lower level handling of sequencing.

To improve DX the execution error response from the painless API was massaged to a more reader friendly state, only giving non-repeating information. Currently it is hard to determine the line and character information from the painless endpoint. If the user wishes to see this raw information it will be available in the API response flyout.

jloleysens · 2020-02-20T15:41:36Z

Unfortunately the position information returned in the Painless execution is not entirely accurate. For invalid syntax it would sometimes be up to 4 chars off, which could include a newline and so we report the wrong line in certain cases.

We could consider adding that to the curated output down the line.

elasticmachine · 2020-02-21T15:51:45Z

Pinging @elastic/es-ui (Team:Elasticsearch UI)

cjcenizal

Awesome changes, @jloleysens! Thanks for cleaning this up. I tested locally and reviewed the code. All looks great. 💪

cjcenizal · 2020-02-21T16:07:02Z

x-pack/legacy/plugins/painless_lab/public/hooks/use_submit_code.ts

+const DEBOUNCE_MS = 800;
+
+export const useSubmitCode = (http: HttpSetup) => {
+  // .then off the same promise reference to enforce sequential


Is this comment still needed?

cjcenizal · 2020-02-21T16:07:22Z

x-pack/legacy/plugins/painless_lab/public/components/output_pane/output_tab.tsx

      <EuiSpacer size="m" />
      <EuiCodeBlock language="json" paddingSize="s" isCopyable>
-        {formatResponse(response.success || response.error)}
+        {formatResponse(response ?? undefined)}


Nit: if we initialize response as undefined in useSubmitCode, then would we need ?? undefined here or need to change the Props interface? I have a hard time recalling an instance where I got value out of a null value and so I've generally been defaulting to things being undefined unless there is a really good reason to use null. So far it's seemed to help simplify code in my experience.

cjcenizal · 2020-02-21T16:12:47Z

x-pack/legacy/plugins/painless_lab/public/components/output_pane/output_pane.tsx

            id: 'output',
-            name: outputTabLabel,
+            // TODO: Currently this causes an Eui prop error because it is expecting string, but we give it React.ReactNode - should fix.
+            name: outputTabLabel as any,


This should be a pretty straightforward fix in EUI. The underlying EuiTab component already accepts a node, so it should be a one-liner fix to change EuiTabbedContent to accept a node too. There may be Jest snapshots that also require updating.

Stick with "undefined" as the designation for something not existing.

kibanamachine · 2020-02-24T11:43:37Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request
Commit: e44a913

History

💛 Build #28100 was flaky 4604e1c
💔 Build #27914 failed f9f04cd

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

* Create Painless Playground app (#54578) * Replace heart script with smiley face script. (#57755) * Rename Painless Playground -> Painless Lab. (#57545) * Fix i18n namespace. * Improve smiley face proportions. - Add def keyword to Painless spec. - Temporarily fix broken highlighting. - Add small padding to main controls. * [Painless Lab] Minor Fixes (#58135) * Code restructure, improve types, add plugin id, introduced hook Moved the code execution hook to a custom hook outside of main, also chaining off promise to avoid lower level handling of sequencing. * Re-instated formatting code To improve DX the execution error response from the painless API was massaged to a more reader friendly state, only giving non-repeating information. Currently it is hard to determine the line and character information from the painless endpoint. If the user wishes to see this raw information it will be available in the API response flyout. * Remove leading new line in default script * Remove registration of feature flag * Fix types * Restore previous auto-submit request behaviour * Remove use of null and remove old comment Stick with "undefined" as the designation for something not existing. * [Painless Lab] NP migration (#59794) * Fix sample document editor. * [Painless Lab] Fix float -> integer coercion bug (#60201) * Clarify data and persistence flow. Fix floating point precision bug. * Send a string to API and ES client instead of an object. * Rename helpers lib to format. Add tests for formatRequestPayload. * Add query parameter to score context (#60414) * Fix typo and i18n * Make state init lazy Otherwise we are needlessly reading and JSON.parse'ing on every state update * Support the query parameter in requests to Painless * Fix borked i18n * Fix i18n * Another i18n issue * [Painless] Minor state update model refactor (#60532) * Fix typo and i18n * Make state init lazy Otherwise we are needlessly reading and JSON.parse'ing on every state update * Support the query parameter in requests to Painless * WiP on state refactor * Some cleanup after manual testing * Fix types and i18n * Fix i18n in context_tab * i18n * [Painless] Language Service (#60612) * Added language service * Use the correct monaco instance and add wordwise operations * Remove plugin context initializer for now * [Painless] Replace hard-coded links (#60603) * Replace hard-coded links Also remove all props from Main component * Pass the new links object to the request flyout too * Link directly to painless execute API's contexts * Remove responsive stacking from tabs with icons in them. * Resize Painless Lab bottom bar to accommodate nav drawer width (#60833) * Validate Painless Lab index field (#60841) * Make JSON format of parameters field more prominent. Set default parameters to provide an example to users. * Set default document to provide an example to users. * Simplify context's updateState interface. * Refactor store and context file organization. - Remove common directory, move constants and types files to root. - Move initialState into context file, where it's being used. * Add validation for index input. * Create context directory. * Fix bottom bar z-index. * Position flyout help link so it's bottom-aligned with the title and farther from the close button. Co-authored-by: Matthias Wilhelm <matthias.wilhelm@elastic.co> Co-authored-by: Jean-Louis Leysens <jloleysens@gmail.com> Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> Co-authored-by: Alison Goryachev <alison.goryachev@elastic.co>

* Create Painless Playground app (#54578) * Replace heart script with smiley face script. (#57755) * Rename Painless Playground -> Painless Lab. (#57545) * Fix i18n namespace. * Improve smiley face proportions. - Add def keyword to Painless spec. - Temporarily fix broken highlighting. - Add small padding to main controls. * [Painless Lab] Minor Fixes (#58135) * Code restructure, improve types, add plugin id, introduced hook Moved the code execution hook to a custom hook outside of main, also chaining off promise to avoid lower level handling of sequencing. * Re-instated formatting code To improve DX the execution error response from the painless API was massaged to a more reader friendly state, only giving non-repeating information. Currently it is hard to determine the line and character information from the painless endpoint. If the user wishes to see this raw information it will be available in the API response flyout. * Remove leading new line in default script * Remove registration of feature flag * Fix types * Restore previous auto-submit request behaviour * Remove use of null and remove old comment Stick with "undefined" as the designation for something not existing. * [Painless Lab] NP migration (#59794) * Fix sample document editor. * [Painless Lab] Fix float -> integer coercion bug (#60201) * Clarify data and persistence flow. Fix floating point precision bug. * Send a string to API and ES client instead of an object. * Rename helpers lib to format. Add tests for formatRequestPayload. * Add query parameter to score context (#60414) * Fix typo and i18n * Make state init lazy Otherwise we are needlessly reading and JSON.parse'ing on every state update * Support the query parameter in requests to Painless * Fix borked i18n * Fix i18n * Another i18n issue * [Painless] Minor state update model refactor (#60532) * Fix typo and i18n * Make state init lazy Otherwise we are needlessly reading and JSON.parse'ing on every state update * Support the query parameter in requests to Painless * WiP on state refactor * Some cleanup after manual testing * Fix types and i18n * Fix i18n in context_tab * i18n * [Painless] Language Service (#60612) * Added language service * Use the correct monaco instance and add wordwise operations * Remove plugin context initializer for now * [Painless] Replace hard-coded links (#60603) * Replace hard-coded links Also remove all props from Main component * Pass the new links object to the request flyout too * Link directly to painless execute API's contexts * Remove responsive stacking from tabs with icons in them. * Resize Painless Lab bottom bar to accommodate nav drawer width (#60833) * Validate Painless Lab index field (#60841) * Make JSON format of parameters field more prominent. Set default parameters to provide an example to users. * Set default document to provide an example to users. * Simplify context's updateState interface. * Refactor store and context file organization. - Remove common directory, move constants and types files to root. - Move initialState into context file, where it's being used. * Add validation for index input. * Create context directory. * Fix bottom bar z-index. * Position flyout help link so it's bottom-aligned with the title and farther from the close button. Co-authored-by: Matthias Wilhelm <matthias.wilhelm@elastic.co> Co-authored-by: Jean-Louis Leysens <jloleysens@gmail.com> Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> Co-authored-by: Alison Goryachev <alison.goryachev@elastic.co> Co-authored-by: Matthias Wilhelm <matthias.wilhelm@elastic.co> Co-authored-by: Jean-Louis Leysens <jloleysens@gmail.com> Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> Co-authored-by: Alison Goryachev <alison.goryachev@elastic.co>

jloleysens added 4 commits February 20, 2020 16:09

Code restructure, improve types, add plugin id, introduced hook

886d0ed

Moved the code execution hook to a custom hook outside of main, also chaining off promise to avoid lower level handling of sequencing.

Remove leading new line in default script

1fd9d48

Remove registration of feature flag

f9f04cd

jloleysens requested a review from cjcenizal February 20, 2020 15:38

jloleysens added 2 commits February 21, 2020 10:55

Fix types

fc93a19

Restore previous auto-submit request behaviour

4604e1c

cjcenizal added Feature:Dev Tools painless painless release_note:skip Skip the PR/issue when compiling release notes Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more t// labels Feb 21, 2020

cjcenizal approved these changes Feb 21, 2020

View reviewed changes

Remove use of null and remove old comment

e44a913

Stick with "undefined" as the designation for something not existing.

jloleysens merged commit 35de7e0 into elastic:app/painless Feb 24, 2020

jloleysens deleted the minor-fixes branch February 24, 2020 09:45

cjcenizal added the Feature:Painless Lab Dev tool for learning Painless label Mar 23, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Comments

[Painless Lab] Minor Fixes#58135

[Painless Lab] Minor Fixes#58135
jloleysens merged 7 commits intoelastic:app/painlessfrom
jloleysens:minor-fixes

jloleysens commented Feb 20, 2020 •

edited

Loading

Uh oh!

jloleysens commented Feb 20, 2020

Uh oh!

elasticmachine commented Feb 21, 2020

Uh oh!

cjcenizal left a comment

Uh oh!

cjcenizal Feb 21, 2020

Uh oh!

cjcenizal Feb 21, 2020

Uh oh!

cjcenizal Feb 21, 2020

Uh oh!

kibanamachine commented Feb 24, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Comments

Conversation

jloleysens commented Feb 20, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Screenshots

Uh oh!

jloleysens commented Feb 20, 2020

Uh oh!

elasticmachine commented Feb 21, 2020

Uh oh!

cjcenizal left a comment

Choose a reason for hiding this comment

Uh oh!

cjcenizal Feb 21, 2020

Choose a reason for hiding this comment

Uh oh!

cjcenizal Feb 21, 2020

Choose a reason for hiding this comment

Uh oh!

cjcenizal Feb 21, 2020

Choose a reason for hiding this comment

Uh oh!

kibanamachine commented Feb 24, 2020

💔 Build Failed

History

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

jloleysens commented Feb 20, 2020 •

edited

Loading