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.

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

2.x - Expandable rows and actions column #2390

Closed
wants to merge 2 commits into from
Closed

Conversation

@gwin003
Copy link

gwin003 commented Dec 18, 2014

Added ability to expand rows using an actions column in the grid
Actions column contains an edit, delete and expand button
Each of the buttons can be hidden or disabled
Able to supply callbacks for the edit and delete buttons so you can handle functionality in your application
HTML template that shows in the expandable div can be defined inside your ng-grid directive, and has access to the row entity

Note: The action column icons uses FontAwesome, so that will be required to use the buttons, unless the template is overridden.

Review on Reviewable

@c0bra c0bra added the unknown label Dec 18, 2014
@c0bra
Copy link
Member

c0bra commented Dec 22, 2014

Could you squash all these commits? Having 13 separate commits makes it really difficult to review and it's probably not going to get merged.

Thanks

@gwin003
Copy link
Author

gwin003 commented Dec 22, 2014

Will do, I'll take a look at it this afternoon. Thanks.

@gwin003 gwin003 force-pushed the gwin003:2.x branch from 7bf73a3 to c79d223 Dec 29, 2014
@gwin003
Copy link
Author

gwin003 commented Dec 29, 2014

@c0bra I think I did that right? Sorry, I'm not great with git.

@moberemk
Copy link

moberemk commented Jan 19, 2015

This hasn't been touched in a while, but it's functionality which my project could definitely use--any chance this will get merged in soon @c0bra?

@gwin003
Copy link
Author

gwin003 commented Jan 19, 2015

@moberemk Have you taken a look at what I did on https://github.com/gwin003/ng-grid? My main concern is what I did was too specific for my issue.

@PaulL1 PaulL1 added this to the 2.x.next milestone Apr 9, 2015
@JLLeitschuh
Copy link
Contributor

JLLeitschuh commented Jun 29, 2015

Your second commit says what the PR is not what it adds/removes.
Your message for each commit should do (at least some of) the following:
Answer the following questions:

  1. Why is this change necessary?
    This question tells reviewers of your pull request what to expect in the commit, allowing them to more easily identify and point out unrelated changes.
  2. How does it address the issue?
    Describe, at a high level, what was done to affect change. Introduce a red/black tree to increase search speed or Remove , which was causing are good examples. If your change is obvious, you may be able to omit addressing this question.
  3. What side effects does this change have?
    This is the most important question to answer, as it can point out problems where you are making too many changes in one commit or branch. One or two bullet points for related changes may be okay, but five or six are likely indicators of a commit that is doing too many things.

This was taken from here: https://robots.thoughtbot.com/5-useful-tips-for-a-better-commit-message

@faris-git
Copy link

faris-git commented Aug 18, 2016

I'm new to ui-grid. I'm looking for a feature expandable columns with combination of infinite scroll. Is this issue related to new feature which support expanding columns(having sub columns)?

@mportuga
Copy link
Member

mportuga commented Sep 20, 2016

@gwin003 I am closing this PR due to inactivity. I realized this was partially our fault since there weren't a lot of active developers for a bit, but since you weren't able to address @JLLeitschuh's concerns. @faris-git If you would like this feature feel free to open a new PR for it at your leisure. @gwin003 is also free to do the same if he wishes.

@mportuga mportuga closed this Sep 20, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

7 participants
You can’t perform that action at this time.