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

feat(Suggestion): ✨ New component #3032

Merged
merged 19 commits into from
Jan 28, 2025
Merged

feat(Suggestion): ✨ New component #3032

merged 19 commits into from
Jan 28, 2025

Conversation

mimarz
Copy link
Collaborator

@mimarz mimarz commented Jan 21, 2025

New PR after I botched updating the other one with latest from next 🙈

#2832 #2610
resolves #2882

Copy link

changeset-bot bot commented Jan 21, 2025

🦋 Changeset detected

Latest commit: 3d17279

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 4 packages
Name Type
@digdir/designsystemet-css Minor
@digdir/designsystemet-react Minor
@digdir/designsystemet Minor
@digdir/designsystemet-theme Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link
Contributor

github-actions bot commented Jan 21, 2025

Preview deployments for this pull request:

Storybook - 28. Jan 2025 - 12:15

Copy link
Contributor

github-actions bot commented Jan 21, 2025

Coverage Report

Status Category Percentage Covered / Total
🔵 Lines 52.4% 2980 / 5687
🔵 Statements 52.4% 2980 / 5687
🔵 Functions 82.94% 180 / 217
🔵 Branches 76.79% 523 / 681
File Coverage
File Stmts Branches Functions Lines Uncovered Lines
Changed Files
packages/react/src/components/Combobox/Combobox.tsx 87.85% 66.66% 71.42% 87.85% 169-170, 219-222, 264-265, 273, 282-283, 330-331, 439-463, 467-470
packages/react/src/components/Suggestion/Suggestion.tsx 12.82% 100% 0% 12.82% 85-129
packages/react/src/components/Suggestion/SuggestionClear.tsx 9.75% 100% 0% 9.75% 30-60, 67-82
packages/react/src/components/Suggestion/SuggestionEmpty.tsx 28.57% 100% 0% 28.57% 30-41
packages/react/src/components/Suggestion/SuggestionInput.tsx 10% 100% 0% 10% 24-55
packages/react/src/components/Suggestion/SuggestionList.tsx 13.63% 100% 0% 13.63% 39-60
packages/react/src/components/Suggestion/SuggestionOption.tsx 30% 100% 0% 30% 13-20
Generated in workflow #1836 for commit 3d17279 by the Vitest Coverage Report Action

@mimarz mimarz marked this pull request as ready for review January 21, 2025 13:03
@Barsnes
Copy link
Member

Barsnes commented Jan 21, 2025

changeset?

Barsnes
Barsnes previously approved these changes Jan 21, 2025
@mimarz
Copy link
Collaborator Author

mimarz commented Jan 22, 2025

Updated to new version of u-datalist. Will do some testing with @eirikbacker to make sure everything still works before merging.

Comment on lines 26 to 35
/**
* The screen reader announcement for singular suggestion, where %d is the number of suggestions
* @default '%d forslag'
*/
singular?: string;
/**
* The screen reader announcement for plural suggestions, where %d is the number of suggestions
* @default '%d forslag'
*/
plural?: string;
Copy link
Member

Choose a reason for hiding this comment

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

Why can't these be on Suggestion.List?

Copy link
Contributor

Choose a reason for hiding this comment

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

could absolutely be and would be easier, just put it here as it felt natural to group all "config" on same element, but maybe both, and filter should be moved to Suggestion.List? 🤔 you can decide ☺️

Copy link
Member

Choose a reason for hiding this comment

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

Since singular and plural is something you can set on u-datalist by default (though with data attrs), it feels more natural that we also do it that way.
filter is not something you can set on any other element, afaik, so it's fine to leave it where it is, I think

Copy link
Contributor

Choose a reason for hiding this comment

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

Cool :) There is some discussion to add filter possibility to native <datalist> so maybe we should move it too? Feel free to do the changes you see fitting and merge whenever :) (I'm busy bee as usual here now 🙊 )

Copy link
Member

Choose a reason for hiding this comment

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

Since filter is used in both .List and .Input, maybe we should keep it on the root Suggestion?

packages/react/src/components/Suggestion/Suggestion.tsx Outdated Show resolved Hide resolved
@Barsnes Barsnes dismissed their stale review January 27, 2025 09:07

changes have been made since this was accepted

@Barsnes
Copy link
Member

Barsnes commented Jan 27, 2025

I have added @deprecated to the Combobox

@mimarz mimarz requested a review from Barsnes January 28, 2025 10:06
@mimarz
Copy link
Collaborator Author

mimarz commented Jan 28, 2025

Looks to work good here 👍

@mimarz mimarz merged commit 0c86e30 into next Jan 28, 2025
10 checks passed
@mimarz mimarz deleted the feat-suggestion branch January 28, 2025 11:20
mimarz pushed a commit that referenced this pull request Jan 28, 2025
This PR was opened by the [Changesets
release](https://github.com/changesets/action) GitHub action. When
you're ready to do a release, you can merge this and the packages will
be published to npm automatically. If you're not ready to do a release
yet, that's fine, whenever you add more changesets to next, this PR will
be updated.

⚠️⚠️⚠️⚠️⚠️⚠️

`next` is currently in **pre mode** so this branch has prereleases
rather than normal releases. If you want to exit prereleases, run
`changeset pre exit` on `next`.

⚠️⚠️⚠️⚠️⚠️⚠️

# Releases
## @digdir/[email protected]

### Minor Changes

- Rename config option from `--json` to `--config`
([#3012](#3012))

### Patch Changes

- Create & build tokens correctly if support-color is missing
([#3035](#3035))

- Removed predefined `accent` variables added during `tokens build`. Use
`data-color` or generated named color variables instead.
([#3041](#3041))

## @digdir/[email protected]

### Minor Changes

- ✨ New experimental component; **Suggestion**
([#3032](#3032))

### Patch Changes

- **Chip**: Make radio and checkbox smaller
([#3039](#3039))

- **ToggleGroup**: Change height to match other form elements
([#3055](#3055))

- Chip: avoid reassigning custom properties
([#3019](#3019))

- **Tabs**: Add overflow-x scroll to tablist
([#3030](#3030))

- **Table**: Use `border-top` in `tbody` ensure prettier tables when no
`thead` ([#3020](#3020))

- **Tag**: Set `height` in case it is used in `display: flex`
([#3020](#3020))

- `ds-input` will now by default fill width.
([#3020](#3020))

- **Field**: Move `align-items: start` from `.ds-field` to `align-self:
start` on `.ds-input` for easier styling
([#3020](#3020))

## @digdir/[email protected]

### Minor Changes

- ✨ New experimental component; **Suggestion**
([#3032](#3032))

- **ErrorSummary**: Split `.Item` into two components
([#3040](#3040))

    -   `ErrorSummary.Item` renders a `li`
    -   `ErrorSummary.Link` renders the link

- Upgrade to React 19 and other related dependencies
([#2964](#2964))

- **Checkbox**: Rename `validation` to `error`
([#3038](#3038))

- **Radio**: Rename `validation` to `error`
([#3038](#3038))

### Patch Changes

- **ErrorSummary**: Add support for `asChild`
([#3040](#3040))

- fix: Use correct DOM property for Popover API when used in React 19
([#3010](#3010))

## @digdir/[email protected]

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create single mode combobox named Suggestion
3 participants