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

8345188: Support tree-structural pseudo-classes #1652

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

mstr2
Copy link
Collaborator

@mstr2 mstr2 commented Nov 28, 2024

The CSS Selectors specification defines the :root pseudo-class that matches root nodes:
https://www.w3.org/TR/selectors-4/#the-root-pseudo

JavaFX uses the non-standard .root style class for the same purpose. We should also support the :root pseudo-class for increased compatibility of JavaFX CSS with the web specification.

Additionally, we should also support the following child-indexed pseudo-classes:
:first-child
:last-child
:only-child
:nth-child() with arguments even and odd

The An+B [of S]? microsyntax for :nth-child() is not supported, as this would require major changes in JavaFX CSS.

/reviewers 2
/csr


Progress

  • Change must not contain extraneous whitespace
  • Change requires a CSR request matching fixVersion jfx24 to be approved (needs to be created)
  • Commit message must refer to an issue
  • Change must be properly reviewed (2 reviews required, with at least 1 Reviewer, 1 Author)

Issue

  • JDK-8345188: Support tree-structural pseudo-classes (Enhancement - P4)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jfx.git pull/1652/head:pull/1652
$ git checkout pull/1652

Update a local copy of the PR:
$ git checkout pull/1652
$ git pull https://git.openjdk.org/jfx.git pull/1652/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 1652

View PR using the GUI difftool:
$ git pr show -t 1652

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jfx/pull/1652.diff

Using Webrev

Link to Webrev Comment

@bridgekeeper
Copy link

bridgekeeper bot commented Nov 28, 2024

👋 Welcome back mstrauss! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

@openjdk
Copy link

openjdk bot commented Nov 28, 2024

❗ This change is not yet ready to be integrated.
See the Progress checklist in the description for automated requirements.

@openjdk openjdk bot added the rfr Ready for review label Nov 28, 2024
@openjdk
Copy link

openjdk bot commented Nov 28, 2024

@mstr2
The total number of required reviews for this PR (including the jcheck configuration and the last /reviewers command) is now set to 2 (with at least 1 Reviewer, 1 Author).

@openjdk openjdk bot added the csr Need approved CSR to integrate pull request label Nov 28, 2024
@openjdk
Copy link

openjdk bot commented Nov 28, 2024

@mstr2 has indicated that a compatibility and specification (CSR) request is needed for this pull request.

@mstr2 please create a CSR request for issue JDK-8345188 with the correct fix version. This pull request cannot be integrated until the CSR request is approved.

@mlbridge
Copy link

mlbridge bot commented Nov 28, 2024

Webrevs

@kevinrushforth
Copy link
Member

This will require a change to cssref.html to list the :root pseudo-class in the list of pseudo-classes for Parent.

Additionally, changes will likely be needed to the two places where we say that JavaFX doesn't support structural pseudo-classes, here and here, since :root is a structural pseudo-class.

@kevinrushforth kevinrushforth self-requested a review December 2, 2024 20:58
@openjdk openjdk bot removed the rfr Ready for review label Dec 3, 2024
@mstr2 mstr2 changed the title 8345188: Set :root pseudo-class on root node 8345188: Support tree-structural pseudo-classes Dec 3, 2024
@openjdk openjdk bot added the rfr Ready for review label Dec 3, 2024
@mstr2
Copy link
Collaborator Author

mstr2 commented Dec 3, 2024

I've expanded the scope of this enhancement to also include several child-indexed pseudo-classes, which are also structural pseudo-classes.

I specifically didn't include the :empty structual pseudo-class (matching a Parent with no children), as this pseudo-class is alrady being used by ComboBoxListViewSkin. Perhaps we should deprecate the use of standard pseudo-classes in skins?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
csr Need approved CSR to integrate pull request rfr Ready for review
Development

Successfully merging this pull request may close these issues.

2 participants