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

Implemented a few helper methods for different types of selectors #10

Merged
merged 3 commits into from
Feb 8, 2014

Conversation

chriscantu
Copy link
Contributor

I wanted a little simpler API to work with when creating page objects.

I implemented unit tests for each of the methods. Please let me know if you have any questions or if I missed anything.

Thanks!

@chriscantu chriscantu mentioned this pull request Feb 3, 2014
@Droogans
Copy link
Contributor

Droogans commented Feb 3, 2014

If this gets merged, I'd love to look at adding some more aliases, and making the api more "complex" (but still simpler than it is now).

Here is my idea:

module.exports = Page.create({
    url: { value: '/home' },

    btnLogin: {
        return this.find.css('button.navbar-btn')); }
    },

    btnLogout: {
        get: function() { return this.btnLogin; }
    },

    lnkEpikVote: {
        return this.find.css('a.epik-brand')); }
    },

    tblTabs: {
        return this.finds.css('ul.navbar-nav li[ng-show$="loggedIn"] a')); }
    }
});

Which would be equivalent to:

module.exports = Page.create({
    url: { value: '/home' },

    btnLogin: {
        return this.findByCss('button.navbar-btn')); }
    },

    btnLogout: {
        get: function() { return this.btnLogin; }
    },

    lnkEpikVote: {
        return this.findByCss('a.epik-brand')); }
    },

    tblTabs: {
        return this.findAllByCss('ul.navbar-nav li[ng-show$="loggedIn"] a')); }
    }
});

@stuplum
Copy link
Owner

stuplum commented Feb 3, 2014

@chriscantu I like the idea of the helpers, although not sure about the consistency of the naming. For example, 'findByCss' is fine as it mirrors the locator strategy (by.css), but 'findSelect' and 'findButtonByText' don't match the locator strategies they delegate to. They may possibly read slightly better, but I wonder if consistency should win out here.

Additionally I'd be interested to hear how many of the other locator strategies are actually being used in the wild. If we decide to add helpers for them all, then there are still quite a few outstanding see https://github.com/angular/protractor/blob/master/docs/api.md#locator-strategies.

@Droogans I'm not sure your suggestion would simplify the api any more, I think the difference between 'find' and 'finds' may be too subtle. If we do go for wrapping all of the existing locator strategies, I do concede that the api may need some finessing.

@chriscantu
Copy link
Contributor Author

Hello @stuplum,

Yes, I was trying to go for better readability. I do get your point about staying consistent with protractor existing strategies.

Instead of the approach I wrote, would it make sense to do something like similar to what @Droogans suggested:

this.find.by.css('className');
this.find.all.by.css('className');
this.find.by.select('selectBinding');
this.find.by.buttonText('Button Text');

The 'all' ensures that is explicit enough to indicate that multiple element will be returned. Additionally, it will mirror the protractor strategies.

Thoughts?

@chriscantu
Copy link
Contributor Author

@stuplum - Would you mind responding to my last suggestion? I prefer deciding on an approach before I write the code for it.

Thanks!

@stuplum
Copy link
Owner

stuplum commented Feb 6, 2014

@chriscantu Sorry for not replying, I was hoping we might get more comments about preferences as to how the api should develop.

Whilst the last suggestion is very readable, it is surely 'more' verbose than the first suggestion. Speaking to a few guys at work (I was hoping they might get involved in the discussion), there was a 50/50 split opinion on which way to go.

@chriscantu
Copy link
Contributor Author

@stuplum I believe the most important aspect when building an API is clarity. If I had to choose between brevity and clarity, I choose clarity every time.

That being said, its code! If we take one approach and find we don't like it, we can change it later. No decision made today is written in stone.

@stuplum
Copy link
Owner

stuplum commented Feb 7, 2014

@chriscantu lets go with clarity, I can't choose which way I prefer to be honest.

@chriscantu
Copy link
Contributor Author

@stuplum Updated the pull-request to make the api more clear.

Finished adding all the selectors on the protractor site. Let me know if you have any changes.

Many thanks in advance.

stuplum added a commit that referenced this pull request Feb 8, 2014
Implemented a few helper methods for different types of selectors
@stuplum stuplum merged commit 6ae2af9 into stuplum:master Feb 8, 2014
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