-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[Lens] Improved range formatter #80132
Merged
dej611
merged 36 commits into
elastic:master
from
dej611:feature/range-alternative-formatter
Oct 27, 2020
Merged
Changes from 30 commits
Commits
Show all changes
36 commits
Select commit
Hold shift + click to select a range
54ed36f
:sparkles: New alternative formatter for ranges
dej611 1904551
:white_check_mark: Add range formatter test for alternative format
dej611 080eb8a
Merge remote-tracking branch 'upstream/master' into feature/range-alt…
dej611 c14ff49
Merge remote-tracking branch 'upstream/master' into feature/range-alt…
dej611 004a580
:lipstick: Adopt new range format template
dej611 5a1224e
:lipstick: Revisit styling for range popover
dej611 fb15462
Merge remote-tracking branch 'upstream/master' into feature/range-alt…
dej611 a058a9c
:recycle: Rework the logic of the formatter to better handle the rang…
dej611 2db3014
:label: fix type issue
dej611 142cbfd
:lipstick: Use the arrow format rather than dash
dej611 8bae6d2
:lipstick: Restored append and prepend symbols
dej611 68b1f46
:lipstick: Picked better arrow symbol
dej611 6557b11
:lipstick: Add missing compressed attribute
dej611 f664fb0
:white_check_mark: Add more tests for range params scenarios
dej611 8d9547c
Merge branch 'master' into feature/range-alternative-formatter
kibanamachine fa51aa7
Update x-pack/plugins/lens/public/indexpattern_datasource/operations/…
dej611 89f82a8
Merge remote-tracking branch 'origin/master' into HEAD
wylieconlon 268d82e
Merge branch 'feature/range-alternative-formatter' of github.com:dej6…
wylieconlon d1cf17b
Merge remote-tracking branch 'origin/master' into HEAD
wylieconlon 0868abe
Merge branch 'master' into feature/range-alternative-formatter
kibanamachine 7b860ff
:ok_hand: Refactor Expression phase based on review feedback
dej611 a6dff88
:bug: Fix default formatter handling
dej611 8395a50
:white_check_mark: Add test for default and override formatter scenarios
dej611 fd2ddfe
:label: Fix type check
dej611 6cf8a8b
:bug: Fix decimals inconsistency with range parameters in specific case
dej611 e265b95
pass through index pattern format unchanged if no Lens format is spec…
flash1293 b6ed4b8
Merge branch 'master' into feature/range-alternative-formatter
kibanamachine 04b391a
:bug: Fix fieldFormatMap serialization issue
dej611 7a93683
:white_check_mark: Add fieldformatMap field to test edge cases
dej611 456a80b
:bug: Better handling to support already serialized formatters
dej611 6241055
Organize the formatting differently
wylieconlon f7fc98b
Merge pull request #2 from wylieconlon/ranges
dej611 45d0933
Merge branch 'master' into feature/range-alternative-formatter
kibanamachine 6e24997
:wrench: Raise the limit for data plugin by 5kb
dej611 bcc2639
Merge branch 'master' into feature/range-alternative-formatter
dej611 adc0ff3
:recycle: Refactor to use async_services
dej611 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -435,7 +435,8 @@ export function DimensionEditor(props: DimensionEditorProps) { | |
/> | ||
)} | ||
|
||
{selectedColumn && selectedColumn.dataType === 'number' ? ( | ||
{selectedColumn && | ||
(selectedColumn.dataType === 'number' || selectedColumn.operationType === 'range') ? ( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would be nice to revisit this conditional logic later, it probably won't scale but it's fine for now. |
||
<FormatSelector | ||
selectedColumn={selectedColumn} | ||
onChange={(newFormat) => { | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a way to abstract this one more level? Ideally the editor_frame_service would contain code that has no references to any specific datasource.
If it's not possible to abstract this another level then maybe we should move the format_column code into the datasource.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I used the same approach as in the https://github.com/elastic/kibana/blob/master/x-pack/plugins/lens/public/indexpattern_datasource/rename_columns.ts#L39 .
From my understanding this function is called from the expression service. Can you confirm?
I can agree on the
format_column
move at datasource level, but probably I'd do in a separate PR.