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.

Sign up for GitHub

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

Add Invoker Commands API #2411

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Add Invoker Commands API #2411

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a9eae6a
Add Invoker Commands API
SilentImp Dec 9, 2024
524149e
Update features/invoker-commands.yml
SilentImp Dec 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions features/invoker-commands.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
name: Invoker Commands
description: The `command` HTML attribute declaratively creates an interaction on a `<button>` element without JavaScript. The interaction target is specified with the `commandFor` HTML attribute on the same `<button>` element.
spec: https://open-ui.org/components/invokers.explainer/
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this PR will be blocked until the spec is merged in whatwg/html#9841, and then we can link to that.

Copy link
Author

Choose a reason for hiding this comment

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

So no explainers or drafts as a spec link?

Copy link
Collaborator

Choose a reason for hiding this comment

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

It's OK to link to other things if there's no better link, but you have to create an exception and document when we ought to remove the exception:

web-features/scripts/specs.ts

Lines 23 to 27 in fe74fdd

const defaultAllowlist: allowlistItem[] = [
// [
// "https://example.com/spec/",
// "Allowed because…. Remove this exception when https://example.com/org/repo/pull/1234 merges."
// ]

Copy link
Contributor

Choose a reason for hiding this comment

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

@ddbeck should this be something we discuss as a potential future improvement? As we head into a new phase of the project where we focus more on bleeding-edge features, we'll very frequently run into this case where features only have early explainers, and no specs.
I have a feeling that adding to the specs.ts file everytime might not scale then.
Maybe open a new discussion issue?

Copy link
Collaborator

@ddbeck ddbeck Dec 12, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Yeah, I think you're right that it won't be scalable. I'd welcome an issue on this. I think it's part of a general question along the lines of, "what's the ideal workflow for features before they ship and standardize?"

Copy link
Contributor

Choose a reason for hiding this comment

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

See #2433

compat_features:
- api.CommandEvent
- api.HTMLElement.command_event
- api.HTMLButtonElement.command
- api.HTMLButtonElement.commandForElement
- html.elements.button.command
- html.elements.button.commandfor


Loading