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

Resize Painless Lab bottom bar to accommodate nav drawer width #60833

Merged
merged 2 commits into from Mar 23, 2020

Conversation

cjcenizal
Copy link
Contributor

Note that there's a bug in master regarding the nav drawer's locked state (#60806) so ignore that if you come across it in testing.

nav_drawer_bottom_bar

@cjcenizal cjcenizal added bug Fixes for quality problems that affect the customer experience Feature:Dev Tools painless painless Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more release_note:skip Skip the PR/issue when compiling release notes labels Mar 20, 2020
@cjcenizal cjcenizal requested review from jloleysens and a team March 20, 2020 23:08
@elasticmachine
Copy link
Contributor

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

@cjcenizal cjcenizal mentioned this pull request Mar 21, 2020
13 tasks
Copy link
Contributor

@jloleysens jloleysens left a comment

Choose a reason for hiding this comment

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

@cjcenizal thanks for adding this improvement!

Would you mind addressing the comments below - regarding some minor changes.

const tearDownApp = renderApp(
element,
{ I18nContext, http, uiSettings, links: getLinks(docLinks) },
{ getIsNavDrawerLocked$ }
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we pass through the chrome dependency in the same object as http, uiSettings etc?

@@ -23,7 +23,8 @@ interface AppDependencies {

export function renderApp(
element: HTMLElement | null,
{ http, I18nContext, uiSettings, links }: AppDependencies
{ http, I18nContext, uiSettings, links }: AppDependencies,
props: MainProps
Copy link
Contributor

Choose a reason for hiding this comment

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

I would suggest keeping Main a propless component as far as possible, if we have the ChromeStart service here we can just add it to our app context to bring this in line with the current pattern?

@cjcenizal
Copy link
Contributor Author

Thanks for the review @jloleysens! I agree with all of the points you mentioned and I've addressed them.

@cjcenizal cjcenizal merged commit 093f524 into elastic:app/painless Mar 23, 2020
@cjcenizal cjcenizal deleted the painless/bottom-bar branch March 23, 2020 17:33
@kibanamachine
Copy link
Contributor

💔 Build Failed

History

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

cjcenizal added a commit that referenced this pull request Mar 23, 2020
* 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>
@cjcenizal cjcenizal added the Feature:Painless Lab Dev tool for learning Painless label Mar 23, 2020
cjcenizal added a commit that referenced this pull request Mar 24, 2020
* 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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Fixes for quality problems that affect the customer experience Feature:Dev Tools Feature:Painless Lab Dev tool for learning Painless 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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants