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

Factor out column manipulation in the doc table #10681

Merged
merged 6 commits into from Apr 3, 2017

Conversation

Projects
None yet
3 participants
@weltenwort
Contributor

weltenwort commented Mar 3, 2017

This refactoring effort turns the implicit manipulation of the set of
columns that was scattered throughout the doc table and the field
chooser into explicit function calls. This brings with it the following
improvements:

  • The column manipulation code is not duplicated (DRY)
  • The controller stays in control (IOC)
  • If the required functions are not provided by the controller,
    manipulation of the columns is disabled.

Additionally, the discover_field now uses a properly isolated scope
instead of accessing inherited properties of the scope.

As a side benefit, this enables us to restrict the ability of the user to change the columns in saved search panels on dashboards. See #9523 for a (currently unresolved) discussion on that topic.

@Stacey-Gammon

This looks awesome Felix! Much improved.

@lukasolson

I left a few tiny nitpicks below, but really this LGTM. Thanks for taking this on!

Show outdated Hide outdated ...lugins/kibana/public/discover/components/field_chooser/discover_field.js
Show outdated Hide outdated ...plugins/kibana/public/discover/components/field_chooser/field_chooser.js
ng-if="canMoveColumnRight(name)"
tooltip-append-to-body="1"
tooltip="Move column to the right"
></i>

This comment has been minimized.

@lukasolson

lukasolson Mar 30, 2017

Member

Soooo much more readable. Thanks 👍

@lukasolson

lukasolson Mar 30, 2017

Member

Soooo much more readable. Thanks 👍

@weltenwort

This comment has been minimized.

Show comment
Hide comment
@weltenwort

weltenwort Mar 31, 2017

Contributor

jenkins, test this (and stop failing arbitrarily)

Contributor

weltenwort commented Mar 31, 2017

jenkins, test this (and stop failing arbitrarily)

weltenwort added some commits Mar 2, 2017

Factor out column manipulation in the doc table
This refactoring effort turns the implicit manipulation of the set of
columns that was scattered throughout the doc table and the field
chooser into explicit function calls. This brings with it the following
improvements:

* The column manipulation code is not duplicated (DRY)
* The controller stays in control (IOC)
* If the required functions are not provided by the controller,
  manipulation of the columns is disabled.

Additionally, the `discover_field` now uses a properly isolated scope
instead of accessing inherited properties of the scope.

@weltenwort weltenwort merged commit 18e0252 into elastic:master Apr 3, 2017

1 check passed

kibana-ci Build finished.
Details

weltenwort added a commit to weltenwort/kibana that referenced this pull request Apr 3, 2017

Factor out column manipulation in the doc table (elastic#10681)
* Factor out column manipulation in the doc table

This refactoring effort turns the implicit manipulation of the set of
columns that was scattered throughout the doc table and the field
chooser into explicit function calls. This brings with it the following
improvements:

* The column manipulation code is not duplicated (DRY)
* The controller stays in control (IOC)
* If the required functions are not provided by the controller,
  manipulation of the columns is disabled.

Additionally, the `discover_field` now uses a properly isolated scope
instead of accessing inherited properties of the scope.

* Make filter addition and removal in tests more reliable

* Change function name to plural, move up ng-if

* Remove inconsistent variable initialization

* Fix function name typo

* Save the state in the action instead of a $watch

weltenwort added a commit that referenced this pull request Apr 3, 2017

Factor out column manipulation in the doc table (#11006)
Backports PR #10681

* Factor out column manipulation in the doc table

This refactoring effort turns the implicit manipulation of the set of
columns that was scattered throughout the doc table and the field
chooser into explicit function calls. This brings with it the following
improvements:

* The column manipulation code is not duplicated (DRY)
* The controller stays in control (IOC)
* If the required functions are not provided by the controller,
  manipulation of the columns is disabled.

Additionally, the `discover_field` now uses a properly isolated scope
instead of accessing inherited properties of the scope.

* Make filter addition and removal in tests more reliable

* Change function name to plural, move up ng-if

* Remove inconsistent variable initialization

* Fix function name typo

* Save the state in the action instead of a $watch

Dreadnoth added a commit to Dreadnoth/kibana that referenced this pull request Aug 8, 2017

Factor out column manipulation in the doc table (elastic#11006)
Backports PR elastic#10681

* Factor out column manipulation in the doc table

This refactoring effort turns the implicit manipulation of the set of
columns that was scattered throughout the doc table and the field
chooser into explicit function calls. This brings with it the following
improvements:

* The column manipulation code is not duplicated (DRY)
* The controller stays in control (IOC)
* If the required functions are not provided by the controller,
  manipulation of the columns is disabled.

Additionally, the `discover_field` now uses a properly isolated scope
instead of accessing inherited properties of the scope.

* Make filter addition and removal in tests more reliable

* Change function name to plural, move up ng-if

* Remove inconsistent variable initialization

* Fix function name typo

* Save the state in the action instead of a $watch
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment