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(Combobox): 🐛 Click to open/close should now close when open #2184

Merged
merged 8 commits into from
Jul 25, 2024
Merged
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
8 changes: 7 additions & 1 deletion .changeset/config.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,5 +7,11 @@
"access": "restricted",
"baseBranch": "main",
"updateInternalDependencies": "patch",
"ignore": ["@digdir/design-system-react"]
"ignore": [
"@digdir/design-system-react",
"storefront",
"theme",
"figma-plugin",
"@digdir/components"
]
}
5 changes: 5 additions & 0 deletions .changeset/friendly-islands-punch.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@digdir/designsystemet-react": patch
---

fix(Combobox): :bug: Button for toggling open/close should now close when open
12 changes: 10 additions & 2 deletions .github/workflows/release-snapshot.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,10 @@ name: Release Snapshot

on:
workflow_dispatch:
inputs:
tag:
description: 'NPM tag'
default: ''

jobs:
snapshot:
Expand All @@ -11,6 +15,10 @@ jobs:
steps:
- uses: actions/checkout@v4
- uses: ./.github/actions/gh-setup
- name: Build
run: yarn build
- id: variables
run: echo "tag=${{ github.event.inputs.tag != '' && github.event.inputs.tag || github.ref_name }}" >> $GITHUB_OUTPUT
- name: Creating .npmrc
run: |
cat << EOF > "$HOME/.npmrc"
Expand All @@ -20,9 +28,9 @@ jobs:
NPM_TOKEN: ${{ secrets.NPM_TOKEN }}
- name: Create Snapshot Release
run: |
yarn run version-packages --snapshot "${{ github.ref_name }}"
yarn run version-packages --snapshot "${{ steps.variables.outputs.tag }}"
echo '---'
echo 'Detected Changes:'
git diff
echo '---'
yarn run publish --tag "${{ github.ref_name }}" --no-git-tag
yarn run publish --tag "${{ steps.variables.outputs.tag }}" --no-git-tag
Original file line number Diff line number Diff line change
Expand Up @@ -322,8 +322,6 @@ describe('Combobox', () => {
await act(async () => await user.click(combobox));
await act(async () => await user.click(screen.getByText('Leikanger')));

await act(async () => await user.click(combobox));

expect(screen.getByText('Leikanger')).toBeInTheDocument();
expect(screen.getByText('Oslo')).toBeInTheDocument();
expect(screen.getByText('Brønnøysund')).toBeInTheDocument();
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
import { useContext } from 'react';
import { forwardRef, useContext } from 'react';
import { XMarkIcon } from '@navikt/aksel-icons';
import cl from 'clsx/lite';

import { ComboboxContext } from '../ComboboxContext';

const ComboboxClearButton = () => {
const ComboboxClearButton = forwardRef<
HTMLButtonElement,
React.ButtonHTMLAttributes<HTMLButtonElement>
>((props, ref) => {
const context = useContext(ComboboxContext);

if (!context) {
Expand All @@ -15,6 +18,8 @@ const ComboboxClearButton = () => {

return (
<button
{...props}
ref={ref}
disabled={disabled}
className={cl('ds-combobox__clear-button', `ds-focus`)}
onClick={() => {
Expand All @@ -39,7 +44,7 @@ const ComboboxClearButton = () => {
/>
</button>
);
};
});

ComboboxClearButton.displayName = 'ComboboxClearButton';

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import type { ChangeEvent } from 'react';
import { useContext } from 'react';
import { useContext, useRef } from 'react';
import cl from 'clsx/lite';
import { ChevronUpIcon, ChevronDownIcon } from '@navikt/aksel-icons';
import { useMergeRefs } from '@floating-ui/react';
Expand Down Expand Up @@ -32,6 +32,7 @@ const ComboboxInput = ({
}: ComboboxInputProps) => {
const context = useContext(ComboboxContext);
const idDispatch = useComboboxIdDispatch();
const clearButtonRef = useRef<HTMLButtonElement>(null);

if (!context) {
throw new Error('ComboboxContext is missing');
Expand Down Expand Up @@ -89,11 +90,12 @@ const ComboboxInput = ({
'aria-controls': null,
'aria-expanded': null,
'aria-haspopup': null,
/* If we click the wrapper, open the list, set index to first option, and focus the input */
onClick() {
/* If we click the wrapper, toggle open, set index to first option, and focus the input */
onClick(event: React.MouseEvent<HTMLDivElement>) {
if (disabled) return;
if (readOnly) return;
setOpen(true);
if (clearButtonRef.current?.contains(event.target as Node)) return;
setOpen(!open);
setActiveIndex(0);
inputRef.current?.focus();
},
Expand Down Expand Up @@ -152,7 +154,7 @@ const ComboboxInput = ({
</Paragraph>
</div>
{/* Clear button if we are in multiple mode and have at least one active value */}
{showClearButton && <ComboboxClearButton />}
{showClearButton && <ComboboxClearButton ref={clearButtonRef} />}
{/* Arrow for combobox. Click is handled by the wrapper */}
<div className={'ds-combobox__arrow'}>
{open ? (
Expand Down
Loading