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

Default implementation for revealInParent to reduce boilerplate #33

Merged
merged 11 commits into from
Nov 28, 2012

Conversation

MobileSam
Copy link
Contributor

I've created a default implementation to the revealInParent() method of the Presenter in relation to this issue on Google Code

Issue 194: Allowing more flexibility and removing some boiler plate when assigning the default slot

https://code.google.com/p/gwt-platform/issues/detail?id=194&colspec=ID%20Stars%20Type%20Status%20Priority%20Component%20Milestone%20Owner%20Summary

@christiangoudreau
Copy link
Member

👍 lgtm

@branflake2267
Copy link
Contributor

Wow, thanks for the commits. I'm checking it out. :)

* RootPopup will fire a {@link RevealRootPopupContentEvent}.
*/
public enum RevealType {
Root,
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it be possible to also put the docs over the enum? Its handy for hovering over and seeing what the docs are for it in eclipse at least .Not sure how IDEA would do. Thoughts?

/**
 * Root will fire a {@link RevealRootContentEvent}.
 */
  Root, 

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do we keep only the title above the enum "The RevealType define..." and inline all descriptions?

@branflake2267
Copy link
Contributor

Do you think there should be some test cases?

@branflake2267
Copy link
Contributor

Since this looks new, I wonder if an example of new use could be done in example or docs? Thoughts?

@MobileSam
Copy link
Contributor Author

I've also created two pr related to this one that update the tests (no new one) and the samples which are using pretty much all of the differents use cases.

For samples see: #35
For tests see: #34

@branflake2267
Copy link
Contributor

I see I haven't made it there yet. Thanks :) I'm working my way backwards.

@branflake2267
Copy link
Contributor

Do you know if all the tests pass? mvn integration-test

Can you catch the branch up before tests :)

@MobileSam
Copy link
Contributor Author

I just got an error on the constructor of TabContainerPresenter in gwtp-sample-tab. I'll look into this first thing in the morning. I might have to change the new constructor a little bit to make it up for the Object parameter that seem to be called by error.

@branflake2267
Copy link
Contributor

I had errors in there to, but should be fixed. The widgets have to be provided at the moment, and its possible there is something left over from what I was doing or its not caught up.

@branflake2267
Copy link
Contributor

The ginbinder fix has been pushed to little later, meanwhile the widgets I changed to provided. Anyway, let me know I can fix that too.

Update the samples to reflect the changes in the Presenter
Updated the tests to reflect changes in the presenter
branflake2267 added a commit that referenced this pull request Nov 28, 2012
Default implementation for revealInParent to reduce boilerplate
@branflake2267 branflake2267 merged commit f7f96c6 into master Nov 28, 2012
branflake2267 added a commit that referenced this pull request Apr 4, 2014
Default implementation for revealInParent to reduce boilerplate
hpehl pushed a commit to hpehl/GWTP that referenced this pull request Dec 9, 2014
Default implementation for revealInParent to reduce boilerplate


Former-commit-id: 3f64b4a
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants