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

#2087 support @As annotation for page object fields #2088

Merged

Conversation

asolntsev
Copy link
Member

@asolntsev asolntsev commented Dec 22, 2022

... not annotated by @findby, but initialized with $ or $$.

This pull request allows to declare page object fields with $ or $$, but mark them with annotation @As:

static class PageObject {
    @As("Dollar header")
    SelenideElement header4 = $("h1");

    @As("Dollar collection")
    ElementsCollection header5 = $$("h2");
}

@PramodKumarYadav

... not annotated by @findby, but initialized with $ or $$.
Copy link
Contributor

@PramodKumarYadav PramodKumarYadav left a comment

Choose a reason for hiding this comment

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

Looks good to me 🙌!

Only remark: The if-else block in the initFields method have grown a bit lengthy but I dont fully understand the implementation to comment if this could be optimised further (or not).

Considering that the selenide elements block of code only use page and proxyIn parameters to derive everything it needs, maybe there is way to break our implementation into further smaller methods that explain our intention here better? However considering I am not able to provide a better refactor alternative at this moment, this is just a passing thought that I wanted to share with you (in case if you can think of an alternative way - or maybe this is already the best alternative).

For now, thanks a ton for making this feature available on such a short notice! I appreciate it :) .

@sonarcloud
Copy link

sonarcloud bot commented Dec 23, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 2 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@asolntsev
Copy link
Member Author

@PramodKumarYadav Good point, I refactored this method: split it to smaller methods.

There is always a room for improvement. :)

@asolntsev asolntsev merged commit 590002f into master Dec 23, 2022
@asolntsev asolntsev deleted the feature/2087-support-as-annotation-for-dollar-fields branch December 23, 2022 14:39
@PramodKumarYadav
Copy link
Contributor

The refactored code is super clear 🙌. Thanks @asolntsev 😊.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

@As annotation command didn't seem to work with SelenideElement
2 participants