search

cheezy / page-object

Gem to implement PageObject pattern in watir-webdriver and selenium-webdriver
MIT License
653 stars 220 forks source link

cause section collections to return section collections from a handfu… #433

Closed AlexisKAndersen closed 7 years ago

AlexisKAndersen commented 7 years ago

…l of methods instead of just arrays

cheezy commented 7 years ago

Hello Alexis. Tell me about this PR? Also, I am about to make a release today or tomorrow. Did you get ChangeLog entries for all of the updates of the past few days?

Thanks for your help :)

AlexisKAndersen commented 7 years ago

I originally set out to not inherit Array, but mostly because I've heard it's bad. Functionally I just wanted methods like select to return a filtered down section collection instead of an array. There were a few other ways I looked into accomplishing it, but this seemed the cleanest. I was still hoping to put in just a tiny refactor on this. If you would like to see different here, I'm open to opinions.

AlexisKAndersen commented 7 years ago

Also, I will take some time tonight to write up changelog entries. I had meant to do it earlier this week, but got swamped with other work.

titusfortner commented 7 years ago

Why did you switch away from including Enumerable? That's typically the recommended approach. I think using it and implementing a custom #to_a would address your concern.

AlexisKAndersen commented 7 years ago

@titusfortner, are you saying including enumerable, defining #each and #to_a will make the class return itself from methods like #select? Because I'm not seeing that happening. Or are you saying do those in addition to custom implementing methods like #select

titusfortner commented 7 years ago

I'm saying that it appears you are bending over backward to re-implement what you get for free when you include Enumerable. The code history shows it used to be implemented that way, I'm curious why it was removed.

For instance, look at Watir::ElementCollection

#each is implemented with #to_a which is what locates the elements, figures out the relocatable selector for them and makes sure they are in the correct subtype.

I'm not as familiar with page sections or their collections to know what all goes into them, but I think the pattern should be similar.

cheezy commented 7 years ago

@AlexisKAndersen I would like to get a PageObject release out today. Can I help you finish this PR?

AlexisKAndersen commented 7 years ago

I was going to look into the code the @titusfortner linked and see if I could gain any insight from it. If you want, you can roll without this PR. I'm free at ~6. I'll go on and put in my changelog entries then. Sorry for holding you up.

cheezy commented 7 years ago

No apology necessary. I welcome your contributions.

On Aug 3, 2017, at 2:24 PM, Alexis Andersen notifications@github.com wrote:

I was going to look into the code the @titusfortner https://github.com/titusfortner linked and see if I could gain any insight from it. If you want, you can roll without this PR. I'm free at ~6. I'll go on and put in my changelog entries then. Sorry for holding you up.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/cheezy/page-object/pull/433#issuecomment-320065968, or mute the thread https://github.com/notifications/unsubscribe-auth/AACGKkBhIdLSWukfNWuJGGtHndKq6pHcks5sUh5bgaJpZM4On6Ak.

cheezy commented 7 years ago

@AlexisKAndersen I'm going to try to get a new release of the gem out late this week. Any chance of including this update?

AlexisKAndersen commented 7 years ago

Unfortunately, I'm very busy this week. I'll try and tie this up this weekend though.

AlexisKAndersen commented 7 years ago

To be honest, while the pull request could probably be merged in the state it's in, it's not providing a benefit that anyone is asking for right now, and I'm not particularly fond of the current solution. I'm closing for now, but we can revisit if anyone has a dying need for this in the future.