Factor out sorting in the doc table #10924
Merged
Conversation
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 refactoring effort turns the implicit manipulation of the sort order, that was performed inline in the table header, into explicit calls to functions provided by the parent of the `<doc-table>` directive. It thereby separates presentation logic from business logic and prepares the directive for further refactorings. Additionally, the parent can now control whether sorting is allowed at all by either implementing custom logic in the `onSortOrderChange` handler or by leaving it out completely. In the latter case the sorting controls will be completely hidden from the header.
3 tasks
lukasolson
approved these changes
Mar 31, 2017
stacey-gammon
approved these changes
Mar 31, 2017
| @@ -117,6 +117,10 @@ uiModules | |||
| $scope.$watchCollection('panel.sort', function () { | |||
| $scope.saveState(); | |||
| }); | |||
|
|
|||
| $scope.setSortOrder = function setSortOrder(columnName, direction) { | |||
There was a problem hiding this comment.
I wonder if we can get rid of the watch above if we move $scope.saveState() in here. As long as the state is updated from the initial setting above. Do you think getting rid of the watch would increase readability?
There was a problem hiding this comment.
Yes, that's a good idea. Looks like that's the only place where the sorting can be changed. It wouldn't just improve readability but also performance (fewer evaluations during digest). Thank you!
|
@stacey-gammon I implemented your suggestion. Are you fine with the way that looks? |
stacey-gammon
approved these changes
Apr 1, 2017
weltenwort
added a commit
to weltenwort/kibana
that referenced
this pull request
Apr 3, 2017
This refactoring effort turns the implicit manipulation of the sort order, that was performed inline in the table header, into explicit calls to functions provided by the parent of the `<doc-table>` directive. It thereby separates presentation logic from business logic and prepares the directive for further refactorings. Additionally, the parent can now control whether sorting is allowed at all by either implementing custom logic in the `onSortOrderChange` handler or by leaving it out completely. In the latter case the sorting controls will be completely hidden from the header.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This refactoring effort turns the implicit manipulation of the sort
order, that was performed inline in the table header, into explicit
calls to functions provided by the parent of the
<doc-table>directive. It thereby separates presentation logic from business logic
and prepares the directive for further refactorings.
Additionally, the parent can now control whether sorting is allowed at
all by either implementing custom logic in the
onSortOrderChangehandler or by leaving it out completely. In the latter case the sorting
controls will be completely hidden from the header.