Skip to content

Conversation

@SylvainBoilard
Copy link

@SylvainBoilard SylvainBoilard commented Nov 4, 2024

@Drup
Copy link
Member

Drup commented Nov 8, 2024

Thanks !

If I'm reading the spec right, toggle is a valid event for any popover element ... which can be any HTML element. In that case, ontoggle should be a global attribute.

Is that right ? Can you do the modification accordingly ?

@SylvainBoilard SylvainBoilard changed the title Add name and ontoggle attributes for details HTML elements. Add support for the popover API and for the name attribute on <details> elements. Nov 14, 2024
@SylvainBoilard
Copy link
Author

Yes that is absolutely right; I had initially planned to limit the scope of this PR to the details element but following your input I decided to update it to support the popover API.

@SylvainBoilard SylvainBoilard changed the title Add support for the popover API and for the name attribute on <details> elements. Add support for the popover API and for the name attribute on details elements. Nov 14, 2024
@balat
Copy link
Member

balat commented May 2, 2025

Hello @Drup what do you think of the new version?

@smorimoto smorimoto requested a review from Drup May 27, 2025 12:17
@smorimoto smorimoto requested a review from Copilot December 1, 2025 15:48
Copilot finished reviewing on behalf of smorimoto December 1, 2025 15:59
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR adds comprehensive support for the modern HTML Popover API and extends the details element with the name attribute, enabling named exclusive accordions. The implementation follows HTML specification and is consistent with existing library patterns.

Key Changes:

  • Implements the Popover API with popover, popovertarget, and popovertargetaction attributes
  • Adds name attribute to details elements for exclusive accordion groups
  • Introduces toggle and beforetoggle event handlers for popover and details state changes

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
lib/html_types.mli Defines type signatures for new popover attributes (Popover, Popovertarget, Popovertargetaction), events (OnBeforeToggle, OnToggle), and extends details_attrib with Name; adds new variants (Manual, Hide, Show, Toggle) to big_variant
lib/html_sigs.mli Declares function signatures for event handlers (a_onbeforetoggle, a_ontoggle) and attribute constructors (a_popover, a_popovertarget, a_popovertargetaction)
lib/html_f.ml Implements event handlers and attribute constructors using existing library primitives; extends string_of_big_variant to convert new variant values to HTML-compliant strings
CHANGES.md Documents the addition of popover API support and details name attribute

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants