Skip to content
This repository has been archived by the owner on Jun 19, 2020. It is now read-only.

Introduce fillLabels method to fill out a form based on label text. #948

Merged
merged 1 commit into from
Aug 19, 2014

Conversation

botandrose
Copy link
Contributor

Hi folks! I've begun using it to write acceptance tests, and its a much better tool for this purpose than all the others that I've tried in JavaScript-land. So, thank you for CasperJS!

So, from the standpoint of using CasperJS as a tool to write acceptance tests, I have found one area for improvement. The problem is that the methods for interacting with forms and form elements require direct knowledge of the underlying html, specifically the name attribute of the inputs. Since acceptance tests are written from the standpoint of a non-technical user, and non-technical users don't know anything about html, css selectors, etc; associated label text is a better way of calling out form elements. This strategy also reduces the coupling between the acceptance test and the app, promoting refactoring of the html and css.

This branch is an attempt at adding this behavior to CasperJS, in the form of new fillLabels method. What do you guys think?

@botandrose
Copy link
Contributor Author

What do you guys think? Any feedback on this?

@mickaelandrieu
Copy link
Member

A good idea, but I don't have time (for now) to review.
ping @n1k0

@n1k0
Copy link
Member

n1k0 commented Jul 2, 2014

Somehow I missed this PR. Very interesting approach, I'll review this soon :)

@botandrose
Copy link
Contributor Author

👍 Thanks! I've been using it for the last month or so, and its really improved the feel of our acceptance tests.

@mickaelandrieu
Copy link
Member

@n1k0 I may need this one, do you have enough time this week for this ?
@botandrose I'll add docs for this ;)

labels: function(labelText, formSelector, value) {
var label = this.findOne({type: "xpath", path: '//label[text()="' + labelText + '"]'}, form);
if(label && label.htmlFor) {
return this.findAll('#' + label.htmlFor, form);
Copy link
Member

Choose a reason for hiding this comment

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

How this will "react" if the label doesn't have for attribute ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good eye. It currently will not work with labels that are simply wrapping inputs, only labels that use the for attribute. If this PR gets merged, I will push an additional PR in the future to add this functionality.

@botandrose
Copy link
Contributor Author

Thanks for the review, @mickaelandrieu! @n1k0 let me know if there's anything additional I can do to help this get merged.

@mickaelandrieu
Copy link
Member

ping @n1k0 can I merge it or do you want to review this one before ?

@mickaelandrieu
Copy link
Member

@n1k0 did you have time to take a look ?
@botandrose can you rebase master and squash your commits please ? If @n1k0 is unavailable I'll merge this one soon.

@botandrose
Copy link
Contributor Author

@mickaelandrieu Done. Thanks for stewarding this PR along!

mickaelandrieu added a commit that referenced this pull request Aug 19, 2014
Introduce fillLabels method to fill out a form based on label text.
@mickaelandrieu mickaelandrieu merged commit d68f88d into casperjs:master Aug 19, 2014
@mickaelandrieu
Copy link
Member

Thanks @botandrose

mickaelandrieu added a commit that referenced this pull request Aug 26, 2014
mickaelandrieu added a commit that referenced this pull request Aug 26, 2014
mickaelandrieu added a commit that referenced this pull request Aug 26, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants