On Thu, 19 Oct 2023 23:49:30 GMT, Andy Goryachev <ango...@openjdk.org> wrote:

>> Sai Pradeep Dandem has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   8185831: Included space
>
> modules/javafx.graphics/src/test/java/test/javafx/scene/Node_lookup_Test.java 
> line 154:
> 
>> 152:      */
>> 153:     @Test
>> 154:     public void lookupPseudoTest2() {
> 
> the reason I've used different nodes is because the situation you have with 
> existing nodes might give false positive.
> 
> I wanted to make sure that selector "a." will collect nodes with ".a" and 
> ".a:pseudo", but in the standard set of nodes with g and hg nodes - but they 
> have the same :testPseudo1, so it's not exactly equivalent.
> 
> The other option is to add new nodes, one with ".x" and another with 
> ".x:pseudo"
> If you do that, it might be easier to change the javadoc format to do 
> something like this:
> 
> 
>  └─ Pane
>      ├─ VFlow (Pane) .flow
>      │   ├─ ClippedPane (Pane) .content
> 
> (but it's just a very minor suggestion)

Thanks for the clarification. Now I got the point what you are saying about.
Added a new test case to cover this scenario. Used x, y.. naming to make a 
clear separation from previous scenarios.

However do you still prefer to keep the lookupPseudoTest2() scenarios? This is 
almost similar to quickTest() and lookupAllTest() except that all the nodes 
used in this method have pseudo classes set.

-------------

PR Review Comment: https://git.openjdk.org/jfx/pull/1245#discussion_r1366272302

Reply via email to