nightwatchjs / nightwatch

Integrated end-to-end testing framework written in Node.js and using W3C Webdriver API. Developed at @browserstack
https://nightwatchjs.org
MIT License
11.85k stars 1.35k forks source link

Pages vs Repeated UI Elements #729

Closed iabw closed 8 years ago

iabw commented 9 years ago
Overview

There is not an optimal place to put UI elements used across multiple pages. Common examples are header, footer, login form, cart, ad popups, etc.

I don't think that this is directly comparable to the current implementation of Sections & Elements, because these widgets will generally have their own commands, possibly URLs, and are not just nested selectors.

Current Problems

Pages don't require a url property, but if navigate is called without one, an error is thrown. For certain UI elements, they SHOULD have a URL that can be navigated to, but the URL would only be a hash, such as { url: '#login' } (tabs or content sections would be the primary example). Calling .navigate() on a page with such a url does not currently work, and requires custom implementation.

It becomes easy to propagate copy/pasted widget code across pages, since there's not a baked in solution for them, and you end up with something like client.pages[ {any page} ].login()

Or, you lump such widgets together, and they're not guaranteed any organization - client.pages.utility().login()

Or, you do separate them into separate pages based on widget, and the name is still slightly confusing. When seeing something like client.pages.login(). It is very unclear if there is a separate login page with its own url that is missing a navigate() call, or if the login form is expected to be a UI element in the currently displayed page. You would need to rename the login "page" as widget_login or ui_login or some other arbitrary and ugly name - client.pages.ui_login().

Proposal

I propose adding a new type of page object - widgets, interfaces, ui, or some name like that. These would be defined and instantiated by Nightwatch almost identically to pages with a few key differences:

sharadJay commented 9 years ago

I am using a workaround for this very same problem by creating separate page object files for each section. Inside my pageobjects I have created commands which return reference to these sections. In this way I am able to reduce duplication of common elements but again there is no clear structure. My commands when I use sections looks like this

client.page.MainPage().header().someCommand();

or

client.page.MainPage().header().click("@elementOnHeaderPage")

I suggest having a separate folder for these UI components and like we specify command variables we could mention which all UI components should be treated as a part of page object

iabw commented 9 years ago

@sharadJay It's good to know it's not just me who's thought about this.

Do you think it should it be necessary for these UI elements to be "placed into" and referenced from each page they're located on (the way you're doing now)?

If you require it do be on the page object, pages need extra code to know what components they contain.

// I'm assuming that components would have an easy way to get back to their page,
// the same way pages have `.api`.
myTest: function(){
  client.pages.somePage().header().login()
    .page.doStuff();
},
// But they might not be able to, since that would require creating a new instance of
// each component for each page that they're on.
// It gets more verbose if you can't.
myOtherTest: function(){
  client.pages.someOtherPage().header().login();
  client.pages.someOtherPage().doMoreStuff();
}

If there is a top level domain (like client.components.header()) with the option of including and referencing them from parent pages, it seems like you gain several advantages.

// With components able to be referenced individually, no extra code needs to be added to pages
myTest: function(){
  // "components" is shorter than "pages.anyName()"
  client.components.header().login();
  client.pages.somePage().doStuff();
},
myOtherTest: function(){
  // Or you could abstract it into a custom command,
  // easily usable from any page for global components
  client.login()
    .pages.someOtherPage().doMoreStuff();
},
myThirdTest: function(){
  // You should still be able to reference them from pages for explicitness or reflection
  for (var component in client.pages.someThirdPage().components){
    console.log(component.name);
    client.assert.visible(component.selector);
  }
}
iabw commented 9 years ago

After thinking about it some more, I think these components are more analogous to sections than pages. Sections are just components that haven't appeared on other pages yet. I could even see an argument for building all sections as components, so that they can be easily added and removed from page objects.

And thinking of how I've built my pages, they seem to follow this rule, it's just that there are multiple components "hard-wired" into a single page object. Most page commands deal with only a single "component", and the ones that don't are messy, probably because I'm not separating concerns well enough.

Here's a somewhat contrived but real example of an actual page I have. The billing section and fillBillingFields method are currently repeated across multiple pages.

module.exports = {
    url: function(){ return this.api.globals.domain + '/checkout' },

    elements: {
        shippingIsBilling: '.js-shippingIsBilling',
    },

    sections: {
        shipping: {
            selector: '.js-shippingFieldset',
            elements: {
                firstName: '#shippingAddress\\.firstName',
                lastName: '#shippingAddress\\.lastName',
                street1: '#shippingAddress\\.street1',
                street2: '#shippingAddress\\.street2',
                city: '#shippingAddress\\.city',
                country: '#shippingAddress\\.country',
                subCountry: '#shippingAddress\\.subCountry',
                zipOrPostalCode: '#shippingAddress\\.zipOrPostalCode',
                phoneNumber: '#shippingAddress\\.phoneNumber'
            }
        },
        billing: {
            selector: '.js-billingFieldset',
            elements: {
                firstName: '#billingAddress\\.firstName',
                lastName: '#billingAddress\\.lastName',
                country: '#billingAddress\\.country',
                zipOrPostalCode: '#billingAddress\\.zipOrPostalCode',
                phoneNumber: '#billingAddress\\.phoneNumber'
            }
        }
    },

    commands: [{
        fillShippingFields: function() {
            this.section.shipping
                .assert.visible('@firstName')
                .setValue('@firstName', 'Sherbet')
                .setValue('@lastName', 'Jackson')
                .setValue('@street1', '200000 Main Street')
                .setValue('@street2', '')
                .setValue('@city', 'New York')
                .setValue('@subCountry', 'NY')
                .setValue('@zipOrPostalCode', '80085')
                .setValue('@phoneNumber', '555-123-4567')

            return this
        },

        setShippingIsBilling: function(value) {
            return this
                .assert.visible('@shippingIsBilling')
                .setValue('@shippingIsBilling', value || true)
        },

        fillBillingFields: function() {
            this.section.billing
                .assert.visible('@firstName')
                .setValue('@firstName', 'Sherbet')
                .setValue('@lastName', 'Jackson')
                .setValue('@country', 'United States')
                .setValue('@zipOrPostalCode', '80085')
                .setValue('@phoneNumber', '555-123-4567')

            return this
        }
    }]
};

This could be refactored into components and written instead like this.

// shippingComponent.js
module.exports = {
    selector: '.js-shippingFieldset',

    elements: {
        firstName: '#shippingAddress\\.firstName',
        lastName: '#shippingAddress\\.lastName',
        street1: '#shippingAddress\\.street1',
        street2: '#shippingAddress\\.street2',
        city: '#shippingAddress\\.city',
        country: '#shippingAddress\\.country',
        subCountry: '#shippingAddress\\.subCountry',
        zipOrPostalCode: '#shippingAddress\\.zipOrPostalCode',
        phoneNumber: '#shippingAddress\\.phoneNumber'
    },

    commands: [{
        fillFields: function() {
            this.
                .assert.visible('@firstName')
                .setValue('@firstName', 'Sherbet')
                .setValue('@lastName', 'Jackson')
                .setValue('@street1', '200000 Main Street')
                .setValue('@street2', '')
                .setValue('@city', 'New York')
                .setValue('@subCountry', 'NY')
                .setValue('@zipOrPostalCode', '80085')
                .setValue('@phoneNumber', '555-123-4567')

            return this
        }
    }]
};

// billingComponent.js
module.exports = {
    selector: '.js-billingFieldset',

    elements: {
        firstName: '#billingAddress\\.firstName',
        lastName: '#billingAddress\\.lastName',
        country: '#billingAddress\\.country',
        zipOrPostalCode: '#billingAddress\\.zipOrPostalCode',
        phoneNumber: '#billingAddress\\.phoneNumber'
    },

    commands: [{
        fillFields: function() {
            this.
                .assert.visible('@firstName')
                .setValue('@firstName', 'Sherbet')
                .setValue('@lastName', 'Jackson')
                .setValue('@country', 'United States')
                .setValue('@zipOrPostalCode', '80085')
                .setValue('@phoneNumber', '555-123-4567')

            return this
        }
    }]
};

// page.js
module.exports = {
    url: function(){ return this.api.globals.domain + '/checkout' },

    elements: {
        shippingIsBilling: '.js-shippingIsBilling',
    },

    sections: {
        shipping: client.components.shipping
        billing: client.components.billing
    },

    commands: [{
        fillShippingFields: function() {
            this.section.shipping.fillFields()

            return this
        },

        setShippingIsBilling: function(value) {
            return this
                .assert.visible('@shippingIsBilling')
                .setValue('@shippingIsBilling', value || true)
        },

        fillBillingFields: function() {
            this.section.billing.fillFIelds()

            return this
        }
    }]
};
vomchik commented 9 years ago

@beatfactor What you will say about this issue?

beatfactor commented 9 years ago

It could be useful but I don't think it's something we will be focusing on anytime soon. However I'll leave it open for future enhancements or maybe someone else wants to jump on it.

IndraniBiswas commented 8 years ago

this will be really useful. We have a very functionality rich app and defining page objects has quickly gone out of control. Having a way to define page objects in separate files and even in separate directory structure clubbing closely associated components, While still be able to construct a page using these components will be really useful for code maintenance and clarity and keep page objects page much cleaner than whats possible right now.

beatfactor commented 8 years ago

I'm afraid I'm going to close this, since there hasn't been much activity here. For now, this is still not on our roadmap.

JustGoscha commented 8 years ago

Aren't page objects by itself defined as components that drive pages or parts of pages. The actual name "Page Object" is misleading, because it doesn't have to be a whole page. The Page Object pattern should be renamed to Component Driver pattern.

senocular commented 8 years ago

@JustGoscha FWIW, page objects aren't unique to NW. The docs point to this article:

http://martinfowler.com/bliki/PageObject.html

Which also mentions:

There's an argument here that the name "page object" is misleading because it makes you think you should have just one page object per page. Something like "panel object" would be better - but the term "page object" is what's become accepted.

rachelruderman commented 6 years ago

Would be so helpful to have this! Fingers crossed for future development :)

romierez04 commented 6 years ago

+1 !!!!!!!!!!!

VinhVu0412 commented 5 years ago

+1