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

Paginated table directive #2013

Merged
merged 18 commits into from
Nov 26, 2014
Merged

Paginated table directive #2013

merged 18 commits into from
Nov 26, 2014

Conversation

w33ble
Copy link
Contributor

@w33ble w33ble commented Nov 22, 2014

I wanted to have a reusable, paginated table to use for part 2 of #1537, this gives me that.

  • Creates a paginated table directive, highly borrowed from the agg-param-table
  • Uses the new directive in the agg-param-table

@w33ble w33ble added the review label Nov 24, 2014
@w33ble w33ble assigned simianhacker and spalger and unassigned simianhacker Nov 24, 2014
spalger
spalger reviewed Nov 25, 2014
View reviewed changes
src/kibana/components/agg_table/agg_table.js
}

col.class = cls.join(' ');
return col;
Copy link
Contributor

Choose a reason for hiding this comment

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

After chatting with you, it turns out that the paginatedTable doesn't need the entire column object, and simply needs the title. I think it would be more clear if this function returned object literals with the title and/or class set.

@spalger
Copy link
Contributor

spalger commented Nov 25, 2014

LGTM -1 comment

@w33ble w33ble assigned w33ble and unassigned spalger Nov 25, 2014
@w33ble
Copy link
Contributor Author

w33ble commented Nov 25, 2014

I think the "ALL" selector in the paginate directive isn't working in the paginated table.

screenshot 2014-11-24 18 15 11

@w33ble w33ble added review and removed review labels Nov 25, 2014
@w33ble w33ble assigned simianhacker and lukasolson and unassigned w33ble and simianhacker Nov 25, 2014
@w33ble
Copy link
Contributor Author

w33ble commented Nov 25, 2014

I made a minor change to paginate, just wanted to get one more set of eyes on it.

@lukasolson
Copy link
Member

I'm seeing the behavior described in #2031 when I'm on this branch, but not in master.

@lukasolson lukasolson assigned w33ble and unassigned lukasolson Nov 25, 2014
@w33ble w33ble removed the review label Nov 25, 2014
@w33ble
Copy link
Contributor Author

w33ble commented Nov 25, 2014

Sorry for the rebase, but the updating spy table is fixed now.

@w33ble w33ble added the review label Nov 25, 2014
@w33ble w33ble assigned lukasolson and unassigned w33ble Nov 25, 2014
This was referenced Nov 25, 2014
Closed
Merged
@lukasolson
Copy link
Member

LGTM

@lukasolson lukasolson assigned w33ble and unassigned lukasolson Nov 25, 2014
lukasolson added a commit that referenced this pull request Nov 26, 2014
@lukasolson
Merge pull request #2013 from w33ble/page-table
c6a3313 
Paginated table directive
@lukasolson lukasolson merged commit c6a3313 into elastic:master Nov 26, 2014
@w33ble w33ble deleted the page-table branch February 6, 2015 21:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@w33ble w33ble

Labels
review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@w33ble @spalger @lukasolson @simianhacker