Skip to content

feat(Search): use ds-suggestion under the hood to support clear button in pure html - #5126

Open
unekinn wants to merge 26 commits into
mainfrom
feat/web-search-clear
Open

feat(Search): use ds-suggestion under the hood to support clear button in pure html#5126
unekinn wants to merge 26 commits into
mainfrom
feat/web-search-clear

Conversation

@unekinn

@unekinn unekinn commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds a @digdir/designsystemet-web/search module (also imported at @digdir/designsystemet-web) which clears and focuses the sibling <input> when a .ds-search button[type="reset"] is clicked. Bails out when the click was already handled (e.g. by React's Search.Clear).

Checks

Clears and focuses the sibling `<input>` when a `.ds-search button[type="reset"]` is clicked.
Bails out when the click was already handled (e.g. by React's `Search.Clear`).
@changeset-bot

changeset-bot Bot commented Jul 31, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 3d05643

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

This PR includes changesets to release 5 packages
Name Type
@digdir/designsystemet-web Minor
@digdir/designsystemet-react Minor
@digdir/designsystemet-css Minor
@digdir/designsystemet Minor
@digdir/designsystemet-types 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

@github-actions

github-actions Bot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Preview deployments for this pull request:

storybook - 14. Aug 2026 - 07:20

www - 14. Aug 2026 - 07:22

@eirikbacker
eirikbacker marked this pull request as draft July 31, 2026 08:09
@eirikbacker
eirikbacker marked this pull request as ready for review August 4, 2026 07:50
@eirikbacker

eirikbacker commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Note, this solution now looks for a button with type reset and with no content, placed directly after a input with type search. It bails out if the click event is already prevented (i.e. like Suggestion.Clear does).

This makes the functionality independent of CSS (making JSDOM tests happy), but it will also potentially catch non-Designsystemet components, if all conditions are met, which is problematic.

In my opinion, I think we should consider removing the Search component entirely, and rather add a magnifying icon on the input if type is set to search in the Suggestion component.

Maybe we should also as well as supporting a "Søk" button, but I must also admit I personally do not see why the search button is connected to the input. This could be a wrapper component (for example "Toolbar") instead, being able to "connect" buttons and/or inputs placed next to each other if we'd like that. This would not make a "special case" button styling for a single component, but enable our consumers to mix and match as needed – IF we feel like we should ship a connected input-button at all that is..
Is the connected input+button is more of an opinionated visual preference, than a component/functionality everyone probably needs? In Mattilsynet, we intentionally do not connect any fields, but rather keep a small distance, simplifying our code and visual appearance: https://www.mattilsynet.no/ (click "Søk" in the menu to see an example) ☺️

@eirikbacker

Copy link
Copy Markdown
Contributor

Updated the code to use data-search="clear" instead of type="reset" to be more specific in what we target ☺️

@Barsnes

Barsnes commented Aug 4, 2026

Copy link
Copy Markdown
Member
image https://www-pr-5126.victoriousglacier-eb9399b9.norwayeast.azurecontainerapps.io/no/components/docs/search/overview image

@Barsnes Barsnes left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Add a changeset about the internals of react search.clear changing and we are good 🔥

Barsnes
Barsnes previously requested changes Aug 4, 2026
Comment thread packages/css/src/search.css Outdated
@Barsnes

Barsnes commented Aug 4, 2026

Copy link
Copy Markdown
Member

Updated the code to use data-search="clear" instead of type="reset" to be more specific in what we target ☺️

Right, but this would break people who have implemented this themselves, since it would not target their reset button anymore

@eirikbacker
eirikbacker requested review from Barsnes and a lite review from Copilot August 4, 2026 12:32

This comment was marked as outdated.

@eirikbacker
eirikbacker marked this pull request as draft August 5, 2026 08:16
@eirikbacker
eirikbacker requested a lite review from Copilot August 5, 2026 16:29
@eirikbacker eirikbacker changed the title feat(web): add a search module so Search.Clear works without React feat(Search): use ds-suggestion under the hood to support clear button in pure html Aug 5, 2026
@eirikbacker

Copy link
Copy Markdown
Contributor
  • Updated Search to use <ds-suggestion> under the hood
  • Extended suggestion.css to include magnifying glass, and thus removing the need for search.css
  • Updated Search so magnifying glass is not bound to "Søk"-button, but instead type of input, giving more control to the end user
  • Updated suggestion.ts to no longer set a empty placeholder as u-combobox now makes this redundant
  • Simplified the suggestion.css to reduce :has() selectors and thus getting better performance
  • Simplified the places [lang] selectors was used instead og :lang()
  • Question: Should we really provide an ugly linear-gradient based chevron, when button[aria-expanded] does not exist? The browser does not.

This comment was marked as resolved.

This comment was marked as resolved.

@eirikbacker
eirikbacker marked this pull request as ready for review August 5, 2026 17:04
@eirikbacker
eirikbacker dismissed Barsnes’s stale review August 6, 2026 06:04

Rewrote the setup, so need new review

Comment thread packages/css/src/breadcrumbs.css Outdated
Comment thread packages/css/src/pagination.css Outdated
Comment on lines +1 to +10
/* .ds-search is kept for backwards compatibility */
.ds-search {
--dsc-search-padding-inline: var(--dsc-suggestion-clear-gap);
--dsc-search-clear-padding: var(--dsc-suggestion-clear-padding);
--dsc-search-clear-size: var(--dsc-suggestion-clear-size);
--dsc-search-clear-icon-url: var(--dsc-suggestion-clear-icon-url);
--dsc-search-magnifying-icon-url: var(--dsc-suggestion-search-icon-url);
--dsc-search-magnifying-icon-size: var(--dsc-suggestion-search-icon-size);
}
.ds-search,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think it would be cleaner to keep this as a separate file. It will be easier to just delete that file later on.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

But the .ds-search, .ds-suggestion selector needs to be in the same file, and using CSS modules, a file can not reference a className from another file, as classnames are often hashed based on the content of the file, so if anyone does individual imports, this is safer :)

@Barsnes Barsnes Aug 11, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

ok, but removing search.css is a breaking change.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Feel free to add it again :)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Added search.css again, importing suggestion.css, but not including search.css in index.css as the backwards compatibility is only needed when importing individual css files

Comment thread packages/react/src/components/search/search.stories.tsx
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.

4 participants