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

(CSS) missing properties #3308

Closed
waarissyb opened this issue Aug 16, 2021 · 4 comments · Fixed by #3356
Closed

(CSS) missing properties #3308

waarissyb opened this issue Aug 16, 2021 · 4 comments · Fixed by #3356
Labels
bug good first issue Should be easier for first time contributors help welcome Could use help from community language

Comments

@waarissyb
Copy link
Contributor

I noticed some CSS properties not being highlighted properly. I found css-shared.js to be missing some properties. Mainly it concerns CSS Grid properties.

Example of produced highlighting:
hljs112-result

Expected behavior:
hljs112-expected

As CSS is nowadays defined by numerous standards and drafts, it’s debatable what should be added. Currently I’m preparing a pull request for this, and I want to discuss what to add.

My solution
To keep the PR simple, I will only add missing CSS properties to css-shared.js. I’m using https://www.w3.org/Style/CSS/all-properties.en.html as my source, which is more useful than the reference of MDN. The latter isn’t complete and contains more than only properties.

Only properties that are part of a W3C Recommendation or Candidate Recommendation will be added. This choice can be seen as conservative, as some properties already listed (like overflow-x) are part of a specification that’s still an Editor’s Draft.

Because the aural part of CSS 2.1 has been deprecated, I will skip those properties and use the properties from CSS Speech Module instead. Other specifications that I’ll consult are:

  • CSS Basic User Interface Module Level 3 (CSS3 UI)
  • CSS Cascading and Inheritance Level 3
  • CSS Fonts Module Level 3
  • CSS Grid Layout Module Level 1
  • CSS Masking Module Level 1
  • CSS Scroll Snap Module Level 1
  • CSS Shapes Module Level 1
  • CSS Template Layout Module
  • CSS Text Module Level 3
  • CSS Transforms Module Level 1
  • CSS Will Change Module Level 1
  • CSS Writing Modes Level 3
  • CSS Writing Modes Level 4
  • Compositing and Blending Level 1

I will be adding around a hundred properties. Any thoughts on the above?

@waarissyb waarissyb added bug help welcome Could use help from community language labels Aug 16, 2021
@joshgoebel
Copy link
Member

Only properties that are part of a W3C Recommendation or Candidate Recommendation will be added. This choice can be seen as conservative, as some properties already listed (like overflow-x) are part of a specification that’s still an Editor’s Draft.

I'd say if multiple mainstream browser engines support it then it's "ok"... but if we're "adding in bulk" as we are here I'm fine with being more conservative. The idea is for our grammars to be useful for 99% of people, not necessarily exhaustive because often the size difference between those two is quite large.

Please submit your PR and I'll have a look.

@deepto98
Copy link
Contributor

deepto98 commented Oct 9, 2021

@joshgoebel Are any tasks remaining for this fix, if so I can pick them up 😃

@joshgoebel
Copy link
Member

Probably but I think the sticky/tricky part is deciding which are immediately useful without just adding everything in the universe (and bloating the grammar for things no one needs/uses)... so I don't know that we have an exact target as to what needs to be done here... it's a bit nebulous.

@waarissyb
Copy link
Contributor Author

I’ve taken my time to go through the CSS specifications again. Some missing things were added, so I’ll create a PR :)

joshgoebel pushed a commit that referenced this issue Oct 13, 2021
…#3356)

* enh(css): add properties from several W3C (Candidate) Recommendations (#3308)
* fix(css): remove CSS values in list of CSS properties
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug good first issue Should be easier for first time contributors help welcome Could use help from community language
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants