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

Fix headless toggle-group class #1017

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 5 additions & 0 deletions .changeset/old-books-judge.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@qwik-ui/headless': patch
---

FIX: Fix headless toggle-group class
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ export default component$(() => {
useStyles$(styles);
return (
<div class="toggle-container">
<ToggleGroup.Root orientation={'vertical'}>
<ToggleGroup.Root data-orientation={'vertical'}>
<ToggleGroup.Item value="left" aria-label="Left aligned" class="toggle">
Left
</ToggleGroup.Item>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ Pass the `direction` prop to `rtl`.
'Enable looping when navigating with the keyboard. Default to `false`.',
},
{
name: 'orientation',
name: 'data-orientation',
type: '"horizontal" | "vertical"',
description: 'Choose the orientation of the toggle items. Default to "horizontal".',
},
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import type { PropsOf, QRL, Signal } from '@builder.io/qwik';
import { PropsOf, QRL, Signal, useStyles$ } from '@builder.io/qwik';
import { component$, useContextProvider, Slot, useTask$, $ } from '@builder.io/qwik';
import {
toggleGroupRootApiContextId,
Expand All @@ -8,6 +8,7 @@ import {
} from './toggle-group-context';
import { useToggleGroup } from './use-toggle';
import { isBrowser, isServer } from '@builder.io/qwik/build';
import styles from './toggle-group.css?inline';

export type ToggleGroupBaseProps = {
/**
Expand All @@ -22,7 +23,7 @@ type ToggleGroupNavigationProps = {
* horizontal for left/right arrows and vertical for up/down arrows.
* Default to (left-to-right) reading mode.
*/
orientation?: Orientation;
'data-orientation'?: Orientation;
/**
* The reading direction of the toggle group.
* Default to (left-to-right) reading mode.
Expand Down Expand Up @@ -87,17 +88,17 @@ export type ToggleGroupApiProps = (ToggleGroupSingleProps | ToggleGroupMultipleP
export type ToggleGroupRootProps = PropsOf<'div'> & ToggleGroupApiProps;

export const HToggleGroupRoot = component$<ToggleGroupRootProps>((props) => {
useStyles$(styles);
const {
onChange$: _,
disabled = false,
orientation = 'horizontal',
direction = 'ltr',
loop = false,
...divProps
} = props;

const orientation = props['data-orientation'] || 'horizontal';
const commonProps = { role: 'group', 'aria-orientation': orientation, dir: direction };
const orientationClass = orientation === 'vertical' ? 'flex-col' : 'flex-row';

const api = useToggleGroup(props);

Expand Down Expand Up @@ -158,27 +159,27 @@ export const HToggleGroupRoot = component$<ToggleGroupRootProps>((props) => {

const setTabIndexInCSR = $(async () => {
/*
Note: given a "single" toggle group with one item already pressed.
Note: given a "single" toggle group with one item already pressed.
- if we use: const allItems = rootApiContext.itemsCSR.value;
- and we lookup for the currentPressedItems, we will get 2 items (the previous and the current)
For that reason to get the currentPressedItems we use: rootApiContext.getAllItem$()
However to get the firstNotDisabledItem, we need to use rootApiContext.itemsCSR.value (refs directly)
as rootApiContext.getAllItem$() will be "out of order".

Ideally, if rootApiContext.getAllItem$() would be in appropriate order, we could use the same logic
for SSR and CSR.
for SSR and CSR.
In should be the case in v2, so we will refactor so both SSR and CSR will use the same API.


The other solution that I consider was:
to have a similar logic "setTabIndexInCSR" but this time which only use the refs
meaning (rootApiContext.itemsCSR.value) within the "toggle-group-item":
meaning (rootApiContext.itemsCSR.value) within the "toggle-group-item":
useTask$(async ({ track }) => {
if (isServer) return;
track(() => rootApiContext.pressedValuesSig.value);
await setTabIndexInCSR();
});

However, I decide to use that function in Root to avoid execute that same logic X times
(X being the number of items) and the fact that Items are consumers that should work in isolation.
They should not execute logic for other Items. This is what Root should do.
Expand Down Expand Up @@ -215,7 +216,7 @@ export const HToggleGroupRoot = component$<ToggleGroupRootProps>((props) => {
Unfortunately, rootApiContext.itemsCSR.value is empty because in the toggle-group-item
the first useTask is tracking the pressedValue changes.
If we put that task at the bottom, we will get the register itemsRef in rootApiContext.itemsCSR.value.
However it will cause other missbehaviors.
However it will cause other missbehaviors.

Instead the safe way is to populate manually using the "document".
In v2, we will not this all those workarounds as the items will be in order and we will use the same API for both SSR and CSR.
Expand Down Expand Up @@ -275,7 +276,6 @@ export const HToggleGroupRoot = component$<ToggleGroupRootProps>((props) => {
<div
{...divProps}
{...commonProps}
class={`flex ${orientationClass} items-center gap-1`}
data-qui-togglegroup-root
>
<Slot />
Expand Down
13 changes: 13 additions & 0 deletions packages/kit-headless/src/components/toggle-group/toggle-group.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
@layer qwik-ui {
/* Horizontal orientation*/
[data-orientation='horizontal'] {
display: flex;
flex-direction: row;
}

/* Vertical orientation*/
[data-orientation='vertical'] {
display: flex;
flex-direction: column;
}
}
Loading