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

[CLOSED] Replace getParent() by findAncestorWithClass() method #164

Closed
tduchateau opened this issue Jan 28, 2014 · 9 comments
Closed

[CLOSED] Replace getParent() by findAncestorWithClass() method #164

tduchateau opened this issue Jan 28, 2014 · 9 comments
Milestone

Comments

@tduchateau
Copy link
Member

Issue by eruiz from Tuesday Jun 25, 2013 at 17:33 GMT


TagSupport.getParent() method gives us access to parent TableTag as follows:

TableTag parent = (TableTag) getParent();

The problem is getParent() doesn't let us to compose tables as shown below:

<datatables:table ...>
  <c:if ...>
    <datatables:column .../>
  </c:if>
</datatables:table>

To solve it, we can use the method TagSupport.findAncestorWithClass() in spite of getParent() method:

TableTag parent = (TableTag) findAncestorWithClass(this, TableTag.class);
@tduchateau
Copy link
Member Author

Comment by tduchateau from Wednesday Jun 26, 2013 at 07:03 GMT

Indeed, that would be great.
Thanks Enrique!

@tduchateau
Copy link
Member Author

Comment by eruiz from Wednesday Jun 26, 2013 at 07:31 GMT

Thibault, I could do the pull-request asap ... ok?

@tduchateau
Copy link
Member Author

Comment by tduchateau from Wednesday Jun 26, 2013 at 07:53 GMT

Go for it! ;-)

@tduchateau
Copy link
Member Author

Comment by eruiz from Monday Jul 08, 2013 at 16:39 GMT

Hi Thibault, I posted the pull request #7

I don't know if it is right because I think github accumulates all changes, including those done in pull request 6. Please take a look.

@tduchateau
Copy link
Member Author

Comment by tduchateau from Wednesday Jul 10, 2013 at 06:01 GMT

Hi Enrique,

Normally it should work properly when your create a new branch dedicated to a pull request from your dandelion-datatables' master branch but this branch needs to remain untouched (at least up to date with the upstream).

Your issue appears to be the fact that you merged one your pull requests in your master branch. That's why the pull #7 embeds commits of the pull #6.

So, what do we do now? :-)

  1. I'm gonna accept the Add support for horizontal scrolling #6 first and then Dandelion 138 #7 (which comes with ugly commits but anyway)
  2. you absolutely need to clear your master branch or your next pull requests will result in the same issue, i.e. it must be kept synchronized with the dandelion-datatables/master branch.
  3. I'm about to create a 0.9.1-wip branch for the next dev. Please use this one for the next PR

Regards,
Thibault

@tduchateau
Copy link
Member Author

Comment by eruiz from Wednesday Jul 10, 2013 at 07:52 GMT

Hi Thibault, agree.

If you prefer:

  1. Accept PR 6 and discard PR 7
  2. I clean and sync our master branch
  3. I do a new PR over the sync master branch to send the changes introduced in PR 7 plus the code that solves issue 146. It would be great if you can include issue 146 in 0.9 :-)

Regards,

Enrique

@tduchateau
Copy link
Member Author

Comment by tduchateau from Wednesday Jul 10, 2013 at 11:46 GMT

  1. Sounds fine to me
  2. Thanks :-)
  3. OK for the new PR that includes changes for [CLOSED] New issue with URL handling with Thymeleaf #138 but please don't include changes that solve [CLOSED] Enhancement. Hide tfoot with dt:selector #146. So either you make a separated PR for [CLOSED] Enhancement. Hide tfoot with dt:selector #146, or I'll take care of it but it will surely be part of the 0.9.0 ;-)

Thanks Enrique!

@tduchateau
Copy link
Member Author

Comment by eruiz from Thursday Jul 11, 2013 at 06:29 GMT

Done! PR 8 ready

@tduchateau
Copy link
Member Author

Comment by tduchateau from Thursday Jul 11, 2013 at 12:48 GMT

See #8

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant