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

Support multiline Timelion queries #11164

Closed
wants to merge 20 commits into from
Closed

Support multiline Timelion queries #11164

wants to merge 20 commits into from

Conversation

cjcenizal
Copy link
Contributor

@cjcenizal cjcenizal commented Apr 11, 2017 •
edited
Loading

Addresses #8881 and #10556

image

image

image

image

image

@cjcenizal cjcenizal force-pushed the 8881/improvement/timelion-multiline-query branch from ddee6d4 to 92c60cb Compare April 11, 2017 20:01
@tbragin tbragin added Team:Platform-Design Team Label for Kibana Design Team. Support the Analyze group of plugins. Feature:Visualizations Generic visualization features (in case no more specific feature label is available) labels Apr 12, 2017
@cjcenizal cjcenizal force-pushed the 8881/improvement/timelion-multiline-query branch from 92c60cb to d09fca0 Compare April 17, 2017 18:11
This was referenced Apr 17, 2017
Merged
Merged
@cjcenizal cjcenizal force-pushed the 8881/improvement/timelion-multiline-query branch from d09fca0 to 8c0188d Compare April 19, 2017 17:10
@cjcenizal cjcenizal force-pushed the 8881/improvement/timelion-multiline-query branch 5 times, most recently from 5aee243 to 0df7fdb Compare May 4, 2017 21:47
@cjcenizal cjcenizal force-pushed the 8881/improvement/timelion-multiline-query branch 2 times, most recently from e30658c to 57f4cf6 Compare May 10, 2017 16:27
@Bargs
Copy link
Contributor

Bargs commented May 10, 2017

@cjcenizal are you still trying to target this for 5.5, or does #10961 going only into 6.0 also push this PR back?

@cjcenizal
Copy link
Contributor Author

@Bargs You're right, this will only go into 6.0 because it depends upon the PR you linked.

@cjcenizal cjcenizal changed the title [WIP] Support multiline queries [WIP] Support multiline Timelion queries May 18, 2017
@cjcenizal cjcenizal added the Feature:Timelion Timelion app and visualization label May 18, 2017
@cjcenizal cjcenizal mentioned this pull request May 18, 2017
Closed
@cjcenizal cjcenizal force-pushed the 8881/improvement/timelion-multiline-query branch 2 times, most recently from 3a1aea2 to d38b3e6 Compare May 19, 2017 20:28
@cjcenizal cjcenizal changed the title [WIP] Support multiline Timelion queries Support multiline Timelion queries May 23, 2017
@cjcenizal
Refactor timelionExpressionInput to use ng-transclude to accept any t…
4363a4c 
…ype of form control. Fix suggestions in Timelion Visualization.
cjcenizal added 18 commits May 22, 2017 18:19
@cjcenizal
Fix broken reference to input element.
988505b
@cjcenizal
Add support for multi-line queries, selects, and secondary inputs to …
5ec6304 
…LocalNavSearch component of UI Framework.
@cjcenizal
Update LocalNavSearchInput to display a truncated single-line preview…
8a588e8 
…. Update UI Framework CSS to correctly set width of example content area.
@cjcenizal
Change max-height of expandable LocalNavSearchInput to a static amoun…
e45b607 
…t, to allow for suggestions to be listed below.
@cjcenizal
Add boundContentEditable directive. Use a contenteditable div instead…
f452228 
… of input element in Timelion.
@cjcenizal
Interpret shift+enter as a new line.
bb0799c
@cjcenizal
Allow use of up and down arrow keys to traverse multiline expression …
4defbfa 
…input.
@cjcenizal
getCaretOffset doesn't work because Selection and Range APIs ignore n…
c51b4bb 
…ewlines.
@cjcenizal
Implement textarea instead of contenteditable.
edabb04
@cjcenizal
Fix bug: hitting enter now submits the query without adding a newline.
268ce3c
@cjcenizal
Fix bug: disallow user from resizing the textarea. Update UI Framewor…
804be38 
…k example.
@cjcenizal
Remove useless timelion_model.js. Add documentation on why contentedi…
c765d1a 
…table sucks.
@cjcenizal
Fix bug: suggestion scrolling logic.
205798f
@cjcenizal
Fix bug: suggestions list scrolls back to top when the list is reopened.
21ed3d8 
- Move event listeners from container element to the transcluded input.
- Throw error if transcluded element isn't available.
@cjcenizal
Hardcode textarea input into timelion-expression-input.
0b728ba 
- Use Angular directives to handle focus, blur, keydown, and keyup events.
- Also move getCaretOffset and setCaretOffset into the directive definition.
- Hide suggestions when the directive loses focuses.
- Show suggestions when the directive gains focus.
- Preserve focus when the user clicks a suggestion.
@cjcenizal
Move expression input out of LocalNav and into main content area, so …
17ee587 
…it won't overlap with graphs when focused.
@cjcenizal
Remove unused bound-content-editable directive.
d9bf77c
@cjcenizal
Rename directives to be easier to find.
b87eabe
@cjcenizal cjcenizal force-pushed the 8881/improvement/timelion-multiline-query branch from edd607d to b87eabe Compare May 23, 2017 01:22
@cjcenizal
Refine design.
d146296 
- Make suggestions popover charts instead of pushing the down.
- Align controls to top of expression input if the textarea is resized to be tall.- Tweak interval styles to look nice.
@cjcenizal
Copy link
Contributor Author

@snide Thoughts on the changes to the UI?

@cjcenizal
Copy link
Contributor Author

jenkins test this!

ppisljar
ppisljar requested changes May 23, 2017
View reviewed changes
Copy link
Member

@ppisljar ppisljar left a comment

Choose a reason for hiding this comment

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

seems this PR contains 2 approaches, where the first one is later thrown away and replaced by the second approach. This makes reviewing this much harder than it should be. Could you rebase this and throw away the commits that are no longer relevant ?

src/core_plugins/timelion/public/chain.peg
@@ -91,7 +91,7 @@ function_name
= first:[a-zA-Z]+ rest:[.a-zA-Z0-9_-]* { return first.join('') + rest.join('') }

function "function"
= space? '.' name:function_name space? '(' space? arg_list:arg_list? space? ')' {
Copy link
Member

Choose a reason for hiding this comment

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

i don't really get it ... how does this solve 'hitting enter now submits the query without adding a newline' ?

ui_framework/components/local_nav/_local_search.scss Outdated
@@ -70,6 +71,7 @@
white-space: nowrap; /* 4 */
overflow: hidden; /* 4 */
text-overflow: ellipsis; /* 4 */
resize: none; /* 5 */
Copy link
Member

Choose a reason for hiding this comment

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

what other text areas does this affect ?

src/core_plugins/timelion/public/app.js Outdated
@@ -23,7 +23,6 @@ const app = require('ui/modules').get('apps/timelion', []);

require('plugins/timelion/services/saved_sheets');
require('plugins/timelion/services/_saved_sheet');
import './directives/timelion_model';
Copy link
Member

Choose a reason for hiding this comment

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

is this is something you added previously in this PR ?

@cjcenizal
Copy link
Contributor Author

Per @ppisljar's request I'm going to clean up the history for easier review. But I'd like to keep this PR as a reference for future contenteditable work. So I'm closing this PR and I'll open a new one with the clean history in its place.

@cjcenizal cjcenizal closed this May 23, 2017
@cjcenizal cjcenizal mentioned this pull request May 23, 2017
Merged
@cjcenizal cjcenizal deleted the 8881/improvement/timelion-multiline-query branch May 23, 2017 16:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ppisljar ppisljar ppisljar requested changes

@rashidkpc rashidkpc Awaiting requested review from rashidkpc

@thomasneirynck thomasneirynck Awaiting requested review from thomasneirynck

Assignees
No one assigned
Labels
Feature:Timelion Timelion app and visualization Feature:Visualizations Generic visualization features (in case no more specific feature label is available) release_note:enhancement review Team:Platform-Design Team Label for Kibana Design Team. Support the Analyze group of plugins. v6.0.0-alpha2 v6.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@cjcenizal @Bargs @ppisljar @epixa @tbragin