Skip to content

[Aksel.nav.no] ✨ Use ctrl + k for search-shortcut #3660

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

Merged
merged 1 commit into from
Mar 14, 2025
Merged

Conversation

KenAJoh
Copy link
Collaborator

@KenAJoh KenAJoh commented Mar 14, 2025

Description

https://nav-it.slack.com/archives/C7NE7A8UF/p1741940197954479

Component Checklist 📝

  • JSDoc
  • Examples
  • Documentation / Decision Records
  • Storybook
  • Style mappings (@navikt/core/css/config/_mappings.js)
  • Component tokens (@navikt/core/css/tokens.json)
  • CSS class deprecations (@navikt/aksel-stylelint/src/deprecations.ts)
  • Exports (@navikt/core/react/src/index.ts and @navikt/core/react/package.json)
  • New component? CSS import (@navikt/core/css/index.css)
  • Breaking change? Update migration guide. Consider codemod.
  • Changeset (Format: <Component>: <gitmoji?> <Text>. E.g. "Button: ✨ Add feature xyz.")

@KenAJoh KenAJoh requested a review from Copilot March 14, 2025 09:48
Copy link

changeset-bot bot commented Mar 14, 2025

⚠️ No Changeset found

Latest commit: 34769df

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

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

Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR updates the search shortcut functionality to use ctrl+k instead of ctrl+b. It adjusts the key event detection in the shortcut hook and updates the search button's aria attributes and visual labels accordingly.

  • Updated key detection in the shortcut hook to support ctrl+k (while still supporting ctrl+b).
  • Changed aria-keyshortcuts and visible text on the search button to reflect the new shortcut.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
aksel.nav.no/website/components/website-modules/search/hooks/useShortcut.ts Updated keyboard event detection to include "k" alongside "b"
aksel.nav.no/website/components/website-modules/search/parts/SearchButton.tsx Replaced all references of "ctrl+b" with "ctrl+k" in aria attributes and UI text

@JulianNymark
Copy link
Contributor

JulianNymark commented Mar 14, 2025

Burde vi beholde ctrl/cmd+B ? (i tilfelle andre er veldig vant med den nå som vi har hatt det en stund?)

Lol, nvm, gikk ut i fra reviewen til copilot over (og den sier "all references" i tabellen... men faktisk riktig i bullet point listen over) ser i koden at det er slik 😂

double edit: nvm, leste ikke tabell entries nøye nok 😳. all is good. (riktig at det er all references innenfor den ene filen)

Copy link
Contributor

@JulianNymark JulianNymark left a comment

Choose a reason for hiding this comment

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

:shipit:

@KenAJoh KenAJoh merged commit e826eb7 into main Mar 14, 2025
6 checks passed
@KenAJoh KenAJoh deleted the header-shortcut branch March 14, 2025 11:41
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.

3 participants