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

Integration: Floating UI React/Svelte (Popups) #1916

Open
Tracked by #2353
endigo9740 opened this issue Aug 21, 2023 · 20 comments
Open
Tracked by #2353

Integration: Floating UI React/Svelte (Popups) #1916

endigo9740 opened this issue Aug 21, 2023 · 20 comments
Assignees
Labels
documentation Improvements or additions to documentation
Milestone

Comments

@endigo9740
Copy link
Contributor

endigo9740 commented Aug 21, 2023

Warning

While we prep and plan for Skeleton v3, we're going to implement a feature freeze on the current popup implementation. This helps prevent redundant effort and avoids pushing out the goalpost even further.

This acts as a centralize HUB for requests related to popups, popovers, drop-downs, tooltips, and similar UI patterns.

The Issue

The Skeleton popup feature was originally implemented to provide very rudimentary popups. This included interacting with a single trigger element that would then show a single popup. Over time, the scope of this feature has grown and users are expecting more and more. To accomplish this, we've opted to integrate Floating UI.

Floating UI’s core is essentially a bunch of mathematical calculations performed on rectangles. These calculations are pure and agnostic, allowing Floating UI to work on any platform that can execute JavaScript.

To keep this dependency optional in Skeleton, users have been required to jump through extra hoops to import Floating UI as a dependency, pass several modules to a dedicated Svelte store, and then all implementation is based around these optional modules. This roundabout means for implementing the dependency has resulted is very limited capabilities, and prevented direct access to Typescript types provided by Floating UI.

The Goal

Seek the simplest and most direct integration path between Floating UI and each framework Skeleton supports, while still providing a friendly interface for common overlay patterns, such as: popovers, drop-down menus, tooltips, context menus, etc.

Solution:

Maintainer Requests

The follow requests are coming from the Skeleton core team and maintains, these highly likely to be implemented:

  • The ability to reuse the same popup with multiple triggers
  • The ability to set or customize the enter/exit animations for the popup
  • Provide new ways to trigger the popup, such as right click context menu
  • Allow popups to be triggered programmatically, rather than using a button or page element.
  • Provide much richer and more prominent type safety
  • Better status updates for when the popup is opened or closed
  • REVIEW: React hook-like pattern for Svelte 5

Community Requests

The following requests come from the Skeleton community and are pending review. They may or may not move forward:

Feedback

If you have additional updates or requests for this feature, please do so in the comments section below.

@baseplate-admin

This comment was marked as resolved.

@endigo9740

This comment was marked as resolved.

@dangodai
Copy link

Suggestion/Feature Request, possibly expanding on The ability to reuse the same popup with multiple triggers depending on what was meant by that: It would be nice if we could pass components and props to the popup action similar to what modals support.

An example to provide some context: Imagine you have a list of items (i.e. an {#each ...} block) and each item has a popup sub-menu full of various functions that need to know about its context such as the ID of the list item.

Apple    [...]   ---click---> __________
Orange   [...]                | Rename |
Banana   [...]                | Move   |
                              | Delete |
                              __________

From a developer UX perspective, it would be nice to use popups in this context along the lines of

<script>
const settings: PopupSettings = { ... }
</script>

{#each fruits as fruit}
    {fruit.name} - <button use:popup={{settings, component: MyPopupComponent, props: { fruit }}}>...</button>
{/each}

Or possibly even have a generator for the popup action along the lines of

<script>
const fruitPopup = GeneratePopupAction({
    event: 'click',
    placement: 'top',
    component: MyPopupComponent // Ideally the generated action would be typed such the the component props are the action parameters
})
</script>

{#each fruits as fruit}
    {fruit.name} - <button use:fruitPopup={{ fruit }}>...</button>
{/each}

@cycle4passion

This comment was marked as off-topic.

@endigo9740

This comment was marked as resolved.

@riziles

This comment was marked as resolved.

@cmjoseph07
Copy link

Suggestion/Feature Request:

Ability to disable/turn off popup(s) through the use of a prop or have popup check the disabled state on the trigger element when open event occurs. An example would be below where we have a popup using the event: 'hover' but we do not want said popup if this element is clicked:

const popupHover: PopupSettings = {
	event: 'hover',
	target: 'popupHover',
	placement: 'top'
};

<button class="btn variant-filled [&>*]:pointer-events-none" use:popup={popupHover}>
	<span>(icon)</span>
	<span>Hover</span>
</button>

Current workaround thanks to Chris would be along the lines of the below where we maintain a local variable for state to control to when to show element:

use:popup={someBool ? popupHover : {}}

@endigo9740
Copy link
Contributor Author

To solve or alongside Joseph's request above, I think we need to check the disabled state of the trigger element. If it's disable the popup shouldn't trigger.

@cycle4passion

This comment was marked as resolved.

@DerrikMilligan
Copy link
Contributor

DerrikMilligan commented Nov 6, 2023

Currently the closeQuery prop doesn't work as outlined in the documentation. It states: You may provide a custom query or set '' to disable this feature.

However, setting closeQuery to '' throws an error in the console.

image

We should probably adjust the onWindowClick event to account for this:

function onWindowClick(event: any): void {
	// Return if the popup is not yet open
	if (popupState.open === false) return;
	// Return if click is the trigger element
	if (triggerNode.contains(event.target)) return;
	// If click it outside the popup
	if (elemPopup && elemPopup.contains(event.target) === false) {
		close();
		return;
	}
	// Handle Close Query State
	const closeQueryString: string = args.closeQuery === undefined ? 'a[href], button' : args.closeQuery;
	// Respect not querying for things if the string is empty
	if (closeQueryString === '') {
		return;
	}
	const closableMenuElements = elemPopup?.querySelectorAll(closeQueryString);
	closableMenuElements?.forEach((elem) => {
		if (elem.contains(event.target)) close();
	});
}

Also I would propose that we could potentially solve some issues with data-popup conflicts by being able to pass in a bind:this reference instead of only a data-popup value.

Example:

<script lang="ts">
// Let the user maintain their own reference to the target
let popupEl: HTMLElement | null = null;

const popupFeatured: PopupSettings = {
	event: 'focus-click',
	// Matches the real element from bind:this
	target: popupEl,
	placement: 'bottom',
};
</script>

<button class="btn variant-filled" use:popup={popupFeatured}>Show Popup</button>

<div class="card p-4 w-72 shadow-xl" bind:this={popupEl}>
	<div><p>Demo Content</p></div>
</div>

Accomplishing this would likely be fairly straightforward as well by modifying the setDomElements method.

function setDomElements(): void {
	// If a real element was passed, use it as the elemPopup
	if (args.target instanceof Element) {
		elemPopup = args.target;
	} else {
		elemPopup = document.querySelector(`[data-popup="${args.target}"]`) ?? document.createElement('div');
	}
	elemArrow = elemPopup.querySelector(`.arrow`) ?? document.createElement('div');
}

@endigo9740

This comment was marked as resolved.

@DerrikMilligan

This comment was marked as resolved.

@endigo9740

This comment was marked as resolved.

@endigo9740 endigo9740 changed the title Popups Refactor NEXT: Popups Refactor Dec 26, 2023
@endigo9740 endigo9740 changed the title NEXT: Popups Refactor NEXT: Popups Updates Dec 26, 2023
@endigo9740 endigo9740 changed the title NEXT: Popups Updates NEXT: Popups Dec 26, 2023
@endigo9740 endigo9740 changed the title NEXT: Popups NEXT: Popups and Floating UI Dec 26, 2023
@endigo9740

This comment was marked as resolved.

@endigo9740 endigo9740 changed the title NEXT: Popups and Floating UI NEXT: Popups (Integration) Jan 3, 2024
@endigo9740 endigo9740 added documentation Improvements or additions to documentation and removed feature request Request a feature or introduce and update to the project. labels Jan 3, 2024
@Sarenor
Copy link
Contributor

Sarenor commented Jan 30, 2024

If the data-popup-id still needs to be unique, we should heavily emphasize that, starting with appending the -id.

@Hugos68
Copy link
Contributor

Hugos68 commented Mar 27, 2024

REPL

POC for a react/vue like hook pattern using svelte runes.

API looks like:

const { refs } = useFloating({
    middleware: [flip()],
    placement: 'top',
});
<button bind:this={refs.reference}>Button</button>
<div bind:this={refs.floating}></div>

@endigo9740
Copy link
Contributor Author

endigo9740 commented Apr 15, 2024

FYI everyone, progress on this issue is starting this week. The goal is to determine how best to handle Floating UI integration for popovers, tooltips, etc. Note that I've updated the "Proposals" section in the original post at the top discussing the ways we can approach this.

I'll aim to provided at least a rudimentary recommendation in a follow up sometime later this week. In the meantime I will be consolidating all requests and feedback to ensure we're provide the best solution long term.

@endigo9740
Copy link
Contributor Author

endigo9740 commented Apr 16, 2024

Still working through this, but wanted to summarize my current thoughts and progress...

React

https://floating-ui.com/docs/react

Overall this provides an extensive toolkit for interacting handling overlay elements in React. It adds ton of critical features, with a number of utilities that provide handy quality of life improvements. I've provided a summary and prioritized list below:

## Critical to Skeleton

- `useFloating` - the core feature, handles most config
- `useInteractions` - opts into event handler hooks, such as:
    - `useHover`
    - `useFocus`
    - `useClick`
- `useRole` - utility to assign ARIA roles to elements
- `useDismiss` - dismiss on request or on click "outside"
- `useTrasition` - allows you to control open/close transitions
- `useClientPoint` - positon floating element at set x/y position
- `FloatingArrow` - handles the floating arrow (pairs with a SVG component)
- `FloatingFocusManager` - focus trap for dialogs

## Nice to Have

- `useListNavigation` - adds arrow navigation to menu lists
- `useTypeahead` - focuses element on typing, NOT for use with inputs or combobox
- `FloatingPortal` - positions floating element outside root/body scope
- `FloatingTree` - context for nested floating elements that are not children
- `FloatingOverlay` - handles backdrops for fades behind dialogs
- `FloatingList` - composable children API for list components
- `FloatingDelayGroup` - handles offset group delays
- `Composite` - (not sure I understand this one)
- `Inner` - allows you to anchor a floating element to a reference centerpoint

Vue

https://floating-ui.com/docs/vue

This is limited to bindings for the core library - for positioning ONLY. It does not offer any features beyond this.

Svelte

https://floating-ui.com/docs/getting-started#vanilla

This would currently lean into the core package, similar to Vue, but without the dedicated bindings.

@endigo9740
Copy link
Contributor Author

endigo9740 commented Apr 23, 2024

We've provided a more in-depth exploration of our proposal for Skeleton v3 popover features here:

We encourage feedback about the proposal be confined to the post linked above.

@endigo9740
Copy link
Contributor Author

Quick update, I encourage everyone to check out our new announcement for Floating UI Svelte. This will act as our long term solution for popovers for Svelte in Skeleton v3:

Additionally, for React users, we'll lean directly into the Floating UI React package:

@endigo9740 endigo9740 unpinned this issue May 17, 2024
@endigo9740 endigo9740 changed the title NEXT: Popups (Integration) Integration: Floating UI React/Svelte (Replaces Popups) May 24, 2024
@endigo9740 endigo9740 changed the title Integration: Floating UI React/Svelte (Replaces Popups) Integration: Floating UI React/Svelte (Popups) May 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

No branches or pull requests

9 participants