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

Solution #266

Closed
wants to merge 9 commits into from
Closed

Solution #266

wants to merge 9 commits into from

Conversation

DimaCoDeeema
Copy link

No description provided.

Copy link
Author

@DimaCoDeeema DimaCoDeeema left a comment

Choose a reason for hiding this comment

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

files deleted

Copy link

@drabykDi drabykDi left a comment

Choose a reason for hiding this comment

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

I left a few comments

};

class ArticlePage extends PageObject {

get commentButton() {

Choose a reason for hiding this comment

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

Add constructor before get commentButton()

constructor(url, slag) {
super(url);
this.url = url + ${slag};
};

Copy link
Author

Choose a reason for hiding this comment

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

Tests don't run after adding. Why we need to add this constructor?

Copy link
Contributor

Choose a reason for hiding this comment

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

task description - ArticlePage constructor takes url and slug. Constructor should modify original url by adding slug at the end

};

class PageObject {

constructor(url) {
this.url = url;

Choose a reason for hiding this comment

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

maybe you should add URL
like this :
this.url = 'http://test.com';

Copy link

@drabykDi drabykDi left a comment

Choose a reason for hiding this comment

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

Good!
Need small fix

Copy link
Contributor

@djtyrf312 djtyrf312 left a comment

Choose a reason for hiding this comment

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

check the comments

Comment on lines +14 to +16
clickOnLogo() {
return 'Click on the Conduit logo';
};
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
clickOnLogo() {
return 'Click on the Conduit logo';
};
clickOnLogo() {
return 'Click on the Conduit logo';
};

task description - PageObject should contain clickOnLogo method. The method should return a string 'Click on the' + logo from the Header class

Copy link
Contributor

Choose a reason for hiding this comment

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

you don't use header class here

};

class ArticlePage extends PageObject {

get commentButton() {
Copy link
Contributor

Choose a reason for hiding this comment

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

task description - ArticlePage constructor takes url and slug. Constructor should modify original url by adding slug at the end

Comment on lines +23 to +25
clickOnCommentButton() {
return 'Click on the [Publish comment] button';
};
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
clickOnCommentButton() {
return 'Click on the [Publish comment] button';
};
clickOnCommentButton() {
return 'Click on the [Publish comment] button';
};

task description - ArticlePage should contain clickOnCommentButton method. The method should return a string 'Click on the' + commentButton

@DimaCoDeeema DimaCoDeeema deleted the develop branch January 21, 2024 18:44
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.

3 participants