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

Source spec info from macros for HTML attributes #6333

Closed
wants to merge 2 commits into from

Conversation

teoli2003
Copy link
Contributor

@teoli2003 teoli2003 commented Jun 25, 2021

This is part of #1146.

I sourced specification for HTML attributes using {{Specification}}

A few notes:

  • I'm not sure having HTML/attributes/ pages (and SVG/attributes) is a good thing. I'm not judging this here, I merely addapt them.
  • BCD for input attribute is next to non-existent. I'm not fixing this here.
  • I used several macros for the cases where the attributes are on several elements.
  • Missing spec_url where bcd was present are fixed in Add spec_url to html attributes browser-compat-data#11240
  • If no bcd, I keep the error message: it will be fixed when we add bcd info.

After this PR, there are 0 {{SpecName}}/{{Spec2}} macros left in web/html.

I plan to use the same techniques for the SVG attributes (>200 tables!)

@github-actions

This comment has been minimized.

@teoli2003 teoli2003 marked this pull request as ready for review June 25, 2021 11:27
@teoli2003 teoli2003 requested a review from a team as a code owner June 25, 2021 11:27
@teoli2003 teoli2003 requested review from mirunacurtean and removed request for a team June 25, 2021 11:27
</tr>
</tbody>
</table>
{{Specifications("html.elements.input.accept")}}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does a spec_url for html.elements.input.accept exist in BCD yet?

I don’t see it in the main branch of BCD, nor in the mdn/browser-compat-data#11240 patch

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No. html.elements.input doesn't exist. BCD is broken into html.element.input-type. I could choose one instead…

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@sideshowbarker Do you think I should just remove the input cases in this PR?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ideally, I’d think we’d want to figure out some way to store the necessary data in BCD. For example, for this case we’d add an html/elements/input/accept.json file to BCD.

That’d mean that in the html/elements/input/ directory, we have a mix of files, some of which are for different input types (what we have now) and some of which are for various input attributes (what we’d need to add).

Yeah, I realize that’d be weird — but input is weird to begin with.

But if we don’t do that and we instead drop the input cases from this PR, does that mean also backing out the part of the change that deletes the existing static spec table?

Or does it instead mean that we end up with no spec links in the articles for these input attributes?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But if we don’t do that and we instead drop the input cases from this PR, does that mean also backing out the part of the change that deletes the existing static spec table?

This is the content PR. So I meant removing, for the moment, the {{Specification}} macro and keep the hardcoded table This would match the bcd PR mdn/browser-compat-data#11240. Advantage: we delay the complex cases and do not block the simple cases.

We could do the opposite: adding the spec_urlat relevant places on bcd for input/email (and the other relevant types) and update the parameter of the macro. I suspect that, even if not complicated, that may not be trivial.

My favour goes to the first alternative (removal from PR) – but this is not a strong feeling, I'm ok with both – as we can constrain the future discussions to a specific case, and move forward with the simple cases.

Copy link
Collaborator

Choose a reason for hiding this comment

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

This is the content PR. So I meant removing, for the moment, the {{Specification}} macro and keep the hardcoded table This would match the bcd PR mdn/browser-compat-data#11240. Advantage: we delay the complex cases and do not block the simple cases.

My favour goes to the first alternative (removal from PR)

I agree. So shall we go ahead with that?

@teoli2003
Copy link
Contributor Author

I'm converting this to draft, as there is a conversation to have before (I'll link it here). I don't want this to be merged by error before.

@sideshowbarker
Copy link
Collaborator

Not sure what this PR is waiting on currently but given that it last had any updates on July 15th, it seems like it might benefit from getting some new attention

@teoli2003
Copy link
Contributor Author

I will close this: no way I can do this before we migrate to Markdown.

@teoli2003 teoli2003 closed this Sep 19, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 19, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants