baseplate-admin
commented
1 year ago HI, Can you give me a rough estimate of when this will arrive?
Closed endigo9740 closed 1 week ago
baseplate-admin
commented
1 year ago HI, Can you give me a rough estimate of when this will arrive?
endigo9740
commented
1 year ago Before the heat death of the universe.
dangodai
commented
1 year ago 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
commented
1 year ago Work in progress for sure. I believe I have handled all issues brought up except still working on custom Svelte transitions. Feel free to ignore, or use any/all. updated 10/27/2023 - support for disabling via prop or disabled triggerNode
endigo9740
commented
1 year ago @cycle4passion seems like this has some really interesting features! Definitely something I'd like to reference going forward. Just FYI me and the core team are discussing some long term plans for Skeleton internally. One of those ideas includes breaking key features off to their own dedicated library package.
This could either be maintained by us (Skeleton Labs) or by members of the community.
The thing I keep coming back to is that I really love Floating UI. It's perfect for what it does, but it missing that extra step of actually providing plug and play solutions for key features such as showing/hiding the elements. https://floating-ui.com/
However, they maintain their own React version, which includes examples for things like tooltips, popovers, and inline dialogs: https://floating-ui.com/docs/react-examples
What I'd love to do is follow their lead and create a Svelte equivalent, whatever that looks like. Whether that's a first party solution created in coordination with the Floating UI maintainers, or a home grown third party wrapper library (ex: svelte-floating-ui).
Either way, the closer we can stick to the source, the better this can be. Heck maybe a wrapper isn't needed, perhaps we just need to show folks how to use the vanilla version of Floating UI paired with Skeleton elements?
riziles
commented
1 year ago Work in progress for sure. I believe I have handled all issues brought up except still working on custom Svelte transitions. Feel free to ignore, or use any/all.
I don't see a lot of Skeleton dependencies in here aside from thestorePopup? Seems like this isn't too far away from being a plug and play Svelte wrapper around Floating UI with some nice additional functionality. Maybe you should publish this as an independent package?
cmjoseph07
commented
1 year ago 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 : {}}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
commented
1 year ago updated Tooltip WIP above to handle disabled prop and triggerNode disabled state.
DerrikMilligan
commented
11 months ago 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.
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');
}@DerrikMilligan this is off topic to the refactor, so I'm going to spin this off to a dedicated bug ticket so it can be reviewed and addressed sooner. I'll go ahead and remove your post here. Please track all further progress here:
DerrikMilligan
commented
11 months ago The second half of it was a suggestion that could maybe be part of the re-factor, not a bug. Perhaps I should have made two comments. Sorry for posting the issue here. I was trying to figure out the right place to put it. In the opening post it said: The above list may grow and evolve as new requests come in. To avoid this extra step, we recommend folks append all new requests or file bugs in this thread via the comments below. Thanks for your cooperation! So I thought this was the place for current bugs too! Haha my bad.
endigo9740
commented
11 months ago You were correct to post here, but given the impact of the bug I thought it best to break it off and consider addressing asap. It's an exception to the rules, so all good. You did everything properly.
endigo9740
commented
10 months ago Note that I've updated the original post to more accurately sum up our expectations for the implementation of this feature in Skeleton v3 (Next). If you have new questions or comments regarding this, please feel free to add them in this comments section. Thank you!
Sarenor
commented
9 months ago If the data-popup-id still needs to be unique, we should heavily emphasize that, starting with appending the -id.
Hugos68
commented
7 months ago 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>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
commented
6 months ago Still working through this, but wanted to summarize my current thoughts and progress...
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 centerpointhttps://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.
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
commented
6 months ago 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
commented
5 months ago 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
commented
1 week ago I'll be posting a new Discussions announcement tomorrow that details our solutions for Popovers in v3. Please refer to this PR, or the updated docs Popover Integrations section on the v3 docs.
Note that work on Floating UI Svelte will continue beyond the Skeleton v3 launch. Progress should be tracked on GitHub.
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.
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:
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.