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

ComponentsContainer is not more generic #42

Closed
wants to merge 5 commits into from
Closed

ComponentsContainer is not more generic #42

wants to merge 5 commits into from

Conversation

jhuska
Copy link
Member

@jhuska jhuska commented Sep 12, 2012

It was my mistake to declare ComponentsContainer as generic, since it can not have only one generic type. ITs method getContent() is generic and it is enough.

@buildhive
Copy link

@lfryc
Copy link
Member

lfryc commented Sep 18, 2012

Hey Juraj, it's not clear for me: ComponentsContainer says it contains other components.

By my opinion it should say it could return another fragment - either a component or composition of several components.

But I don't think it should return list of components, because as you probably wanted to say, we know nothing about the type of list members.

So what about having ComponentContainer<T> with method <T> getContent() method?

Then you could use it as:

@FindBy PopupComponent<RegistrationForm> registrationPopup;

or

@FindBy PopupComponent<ButtonComponent> warningPopup;

@jhuska
Copy link
Member Author

jhuska commented Sep 24, 2012

Hey Lukas,

I used that approach because I had wanted ComponentContainer which can hold more than one Page Fragment.

For example PopupComponent would return List<NestedElements<?>>, with for example NestedElements<RegistrationForm>, NestedElements<Calendar>, NestedElements<CommandButton> items.

However, I found out that this approach is not type safe :(

I will rework it to your proposition and when someone would need ComponentContainer which will hold more PageFragments, he will just implement ComponentConainer<T, E, ...>.

…eneric, NestedElements class removed, affected api clasess fixed
@buildhive
Copy link

@lfryc
Copy link
Member

lfryc commented Sep 25, 2012

Juraj, is there already test covering PageFragmentsContainer usage?

@lfryc
Copy link
Member

lfryc commented Sep 25, 2012

Btw commits contain some old stuff, could you please rebase on master?

Ideally you should create issue + squash commits just to one.

@jhuska
Copy link
Member Author

jhuska commented Feb 4, 2013

I have replaced this pull request with
#72

It was easier for me to do so.

Closing therefore.

@jhuska jhuska closed this Feb 4, 2013
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.

None yet

3 participants