Remove promise from shopping-cart-widget-component.js and add async/await #1153
Conversation
| @@ -10,96 +10,41 @@ export default class ShoppingCartWidgetComponent extends BaseContainer { | |||
| super( driver, by.className( 'cart-toggle-button' ) ); | |||
There was a problem hiding this comment.
Can you update this to a css selector whilst your here? (It's an old selector added before we try to use CSS selectors exclusively)
| return await driverHelper.clickWhenClickable( | ||
| driver, | ||
| by.css( '.cart-toggle-button' ), | ||
| explicitWait |
There was a problem hiding this comment.
this explicit wait isn't actually necessary as it's the same as the default value - would you remind removing it? (added before we defaulted the value I believe)
There was a problem hiding this comment.
Done. Did the same for driver.
|
|
||
| return d.promise; | ||
| async displayedOpen() { | ||
| return await driverHelper.isElementPresent( this.driver, by.css( 'cart-body' ) ); |
There was a problem hiding this comment.
It's great to have one line, but that selector doesn't look like it should have ever worked as it's not a CSS class selector and there's no cart-body element in the DOM. Would you mind updating this to a proper CSS class selector - you could even re-use the selector from the constructor if you wanted to
There was a problem hiding this comment.
I removed displayedOpen(), and in open() method only click to open shopping cart (since in this flow cart is always closed and there is no need to check is it open, I removed check is it open).
There was a problem hiding this comment.
Sorry, I'm not saying what you did is wrong, I just noticed that by.css( 'cart-body' ) would never had worked I don't think 🤔
There was a problem hiding this comment.
I think it should be by.css( '.cart-body' )
There was a problem hiding this comment.
Oh, I misread it again, I think that makes sense as we shouldn't have to worry about whether it's open 😀
There was a problem hiding this comment.
Yes, I understood. But it crossed my mind this would be more optimal solution? It will work both ways.
|
This is looking promising - if we can get rid of promises like this in pages/components it should make it easier for the tests to convert over to call these components |
alisterscott
added
[Status] Needs Author Reply
and removed
[Status] Needs Review
labels
@alisterscott This relates to the comments regarding shopping-cart-widget-component.js in PR #1123
Without promise or async/await tests will pass, but items from shopping cart won't be removed (specs/wp-manage-domains-spec.js).
After adding async/await to component, tests are still passing and shopping cart is empty.