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

Adopt browser-compat-data to show compatibility in hover / completion #102

Closed
octref opened this issue Jun 4, 2018 · 12 comments
Closed
Assignees
Labels
feature-request Request for new features or functionality
Milestone

Comments

@octref
Copy link
Contributor

octref commented Jun 4, 2018

https://github.com/mdn/browser-compat-data

@octref octref added the feature-request Request for new features or functionality label Jun 4, 2018
@connorshea
Copy link
Contributor

Here's the pseudo-class coverage for mdn/data vs mdn/browser-compat-data, this list probably isn't complete since all the pseudo-classes are sourced from the data repo, and there may be some missing items.

Unless otherwise noted, the name of the selectors in the data repo can be mapped to the browser-compat-data repo by taking their name, stripping the colon, and prepending css.selectors..

For example:

:active => css.selectors.active
:first => css.selectors.first
:first-child => css.selectors.first-child

Psuedo-class coverage:

  • :active
    • ✅data
    • ✅bcd
  • :any()
    • ✅data: This is weird because it only has syntax specifically for the -webkit-/-moz- vendor prefixed versions. Also, it should be a function.
    • ❌bcd
  • :any-link
    • ✅data
    • ✅bcd
  • :checked
    • ✅data
    • ✅bcd
  • :default
    • ✅data
    • ✅bcd
  • :defined
    • ✅data
    • ✅bcd
  • :dir()
    • ✅data
    • ✅bcd
    • Should probably be a function but isn't listed as one, in data it's named :dir and in the bcd it's in dir.json.
  • :disabled
    • ✅data
    • ✅bcd
  • :empty
    • ✅data
    • ✅bcd
  • :enabled
    • ✅data
    • ✅bcd
  • :first
    • ✅data
    • ✅bcd
  • :first-child
    • ✅data
    • ✅bcd
  • :first-of-type
    • ✅data
    • ✅bcd
  • :fullscreen
    • ✅data
    • ✅bcd
  • :focus
    • ✅data
    • ✅bcd
  • :focus-visible
    • ✅data
    • ❌bcd
  • :focus-within
    • ✅data
    • ✅bcd
  • :host
    • ✅data
    • ✅bcd
  • :host()
    • ✅data
    • ✅bcd: Note that this is called hostfunction.json and the data mapping in the bcd repo would be css.selectors.hostfunction, this is probably a bug that should be fixed.
  • :host-context
    • ✅data
    • ❌bcd
  • :host-context()
  • :hover
    • ✅data
    • ✅bcd
  • :in-range
    • ✅data
    • ✅bcd
  • :indeterminate
    • ✅data
    • ✅bcd
  • :invalid
    • ✅data
    • ✅bcd
  • :lang()
    • ✅data
    • ✅bcd
    • Should probably be a function but isn't listed as one, in data it's named :lang and in the bcd it's in lang.json.
  • :last-child
    • ✅data
    • ✅bcd
  • :last-of-type
    • ✅data
    • ✅bcd
  • :left
    • ✅data
    • ✅bcd
  • :link
    • ✅data
    • ✅bcd
  • :not()
    • ✅data
    • ✅bcd
    • Should probably be a function but isn't listed as one, in data it's named :not and in the bcd it's in not.json.
  • :nth-child()
    • ✅data
    • ✅bcd
    • Should probably be a function but isn't listed as one.
  • :nth-last-child
    • ✅data
    • ✅bcd
    • Should probably be a function but isn't listed as one.
  • :nth-last-of-type()
    • ✅data
    • ✅bcd
    • Should probably be a function but isn't listed as one.
  • :nth-of-type()
    • ✅data
    • ✅bcd
    • Should probably be a function but isn't listed as one.
  • :only-child
    • ✅data
    • ✅bcd
  • :only-of-type
    • ✅data
    • ✅bcd
  • :optional
    • ✅data
    • ✅bcd
  • :out-of-range
    • ✅data
    • ✅bcd
  • :placeholder-shown
    • ✅data
    • ✅bcd
  • :read-only
    • ✅data
    • ✅bcd
  • :read-write
    • ✅data
    • ✅bcd
  • :required
    • ✅data
    • ✅bcd
  • :right
    • ✅data
    • ✅bcd
  • :root
    • ✅data
    • ✅bcd
  • :scope
    • ✅data
    • ✅bcd
  • :target
    • ✅data
    • ✅bcd
  • :valid
    • ✅data
    • ✅bcd
  • :visited
    • ✅data
    • ✅bcd

@connorshea
Copy link
Contributor

connorshea commented Jun 5, 2018

I looked through css-schema.xml and found a few that are missing from both mdn/data and mdn/browser-compat-data:

  • :blank
  • :corner-present
  • :current
  • :current()
  • :decrement
  • :double-button
  • :decrement
  • :end
  • :future
  • :horizontal
  • :increment
  • :local-link
  • :local-link()
  • :matches()
  • :no-button
  • :nth-column()
  • :nth-last-column()
  • :nth-last-match()
  • :nth-match()
  • :past
  • :recto
  • :single-button
  • :start
  • :user-invalid
  • :verso
  • :vertical
  • :window-inactive

Most of these are non-standard. I also excluded quite a few that are vendor prefixed. I doesn't look like any of them have MDN pages either.

I opened an issue to get these added: mdn/data#241

@connorshea
Copy link
Contributor

There is one other consistency, and that's that the bcd data lists :host as css.selectors.host, :host() as css.selectors.hostfunction, :host-context() as css.selectors.host-context, has no entry for :host-context.

For psuedo-class function selectors, there's not much consistency. Just assuming that :host() maps to host won't work in that specific case, unfortunately. This is probably something we should bring up upstream.

@connorshea
Copy link
Contributor

For properties, you can map anything in properties.json to css.properties.property-name in the browser-compat-data repo, at least as far as I can tell.

There are probably a few properties missing from each dataset, but for the most part it should map cleanly.

Examples:

@connorshea
Copy link
Contributor

@octref so it looks like things should map between the two datasets pretty well ^_^

@octref
Copy link
Contributor Author

octref commented Jun 5, 2018

@connorshea Thanks a lot for your hard work! Those information will definitely come useful when I add support for browser compat data.

As for the nonstandard pseudo selectors, I don't think they should end up in mdn/data because MDN then also need pages dedicated to explaining them. For us we can consider either:

  • Removing those completely
  • Always rank them (and the vendor-prefixed ones) lower than the standard ones

@octref octref added this to the June 2018 milestone Jun 7, 2018
@octref
Copy link
Contributor Author

octref commented Jun 8, 2018

Some notes while studying the data:

Ideas

  • We should show which browser is NOT supported, for standard properties. For example, grid is NOT supported in only IE.

Thoughts

  • We should first study what users want the hover / completion to display regarding browser compatibilities, before rushing to implement something.
  • We need to take into consideration of popular build tools such as autoprefixer, which
    • Prefixes a value if it's needed for the targeted browser range
    • Removes a value if it's not needed for the targeted browser range

Actionable

  • For all experimental values, we add browser-compat data to show compatibility
  • For all other values that don't contain compatibility data, try to look it up in mdn/browser-compat-data and show them

@connorshea
Copy link
Contributor

Also worth noting for the BCD data: there’s an implicit assumption that support data is ordered such that the most recent support data for a browser is at the top (e.g. Chrome 57 is true, then Chrome 55 has the feature behind a flag). So you should be able to parse just the first support entry for each browser, assuming you don’t need any info on prefixes for past versions, etc.

Unfortunately this isn’t enforced in the codebase by any automated tools, though there is an issue to make it happen: mdn/browser-compat-data#1596

@octref
Copy link
Contributor Author

octref commented Jun 11, 2018

@aeschli I pushed a WIP PR. See #105

I have some questions regarding the presentation of the browser compat data. I would like to display it this way:

  • For status: standard properties
    • If it's supported in all our 6 supported browsers, show nothing
    • If any browser does not support it, show unsupported browsers
  • For status: experimental properties: Show both supported and unsupported browsers

I feel the current display is too much noise. Don't believe any user is interested to learn that border is supported from E12,FF1,S1,C1,IE4,O3.5. We should try to highlight which browser don't have support for certain properties.

@aeschli
Copy link
Contributor

aeschli commented Jun 12, 2018

I'd rather not make that change.

  • no complaints from what I know of
  • by having the version number, it gives you an idea of how long the feature has been implemented. The version gets interesting once it has only been recently added.
  • switching to 'not supported on xy' requires us to maintain the information constantly. Also, it doesn't tell you about the set of browsers that we talk about. There are more browsers around, in particular, embedded browsers which we don't cover.
    The current way of listing lists a set of facts that we know.

@octref
Copy link
Contributor Author

octref commented Jun 12, 2018

@aeschli I'm suggesting that because with the new data, a lot of the "supported in all browsers" properties gets a browsers property. For example:

image

https://github.com/Microsoft/vscode-css-languageservice/pull/105/files

These will be quite noisy and unhelpful.

@octref octref closed this as completed in 444743a Jun 18, 2018
octref added a commit that referenced this issue Jun 18, 2018
Import mdn/browser-compat-data. Fix #102
@octref
Copy link
Contributor Author

octref commented Jun 26, 2018

For verifier: check aed9968 and verify the newly added properties are in the completion documentation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request Request for new features or functionality
Projects
None yet
Development

No branches or pull requests

3 participants