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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Search: Auto focus input elements #61443

Merged
merged 3 commits into from Jan 13, 2023
Merged

Search: Auto focus input elements #61443

merged 3 commits into from Jan 13, 2023

Conversation

ryantxu
Copy link
Member

@ryantxu ryantxu commented Jan 13, 2023

Fixes #59853 and https://github.com/grafana/support-escalations/issues/4793

Removing "autofocus" from everywhere caused the search pages to not let you start typing 馃槵

@ryantxu ryantxu added this to the 9.3.4 milestone Jan 13, 2023
@ryantxu ryantxu requested a review from a team as a code owner January 13, 2023 04:57
@ryantxu ryantxu requested review from yaelleC and removed request for a team January 13, 2023 04:57
@grafanabot
Copy link
Contributor

Hello @ryantxu!
Backport pull requests need to be either:

  • Pull requests which address bugs,
  • Urgent fixes which need product approval, in order to get merged,
  • Docs changes.

Please, if the current pull request addresses a bug fix, label it with the type/bug label.
If it already has the product approval, please add the product-approved label. For docs changes, please add the type/docs label.
If none of the above applies, please consider removing the backport label and target the next major/minor release.
Thanks!

@@ -46,6 +46,7 @@ export const ManageDashboardsNew = React.memo(({ folder }: Props) => {
value={state.query ?? ''}
onChange={(e) => stateManager.onQueryChange(e.currentTarget.value)}
onKeyDown={onKeyDown}
// eslint-disable-next-line jsx-a11y/no-autofocus
Copy link
Member Author

Choose a reason for hiding this comment

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

this is the one on the list that seems debateable... but was already autoFocus 馃し

Copy link
Contributor

@ashharrison90 ashharrison90 left a comment

Choose a reason for hiding this comment

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

lgtm. i'd remove the autoFocus from DashboardSearchModal as it's unnecessary. DashboardSearch will get removed as soon as topnav is live anyway 馃檪

@@ -60,6 +60,8 @@ export function DashboardSearchModal({ isOpen }: Props) {
tabIndex={0}
spellCheck={false}
className={styles.input}
// eslint-disable-next-line jsx-a11y/no-autofocus
autoFocus
Copy link
Contributor

Choose a reason for hiding this comment

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

this doesn't need autoFocus, it already focuses when the modal is opened.

@ryantxu ryantxu enabled auto-merge (squash) January 13, 2023 15:55
@ryantxu ryantxu merged commit ce3a96a into main Jan 13, 2023
@ryantxu ryantxu deleted the auto-focus-search-pages branch January 13, 2023 16:01
grafanabot pushed a commit that referenced this pull request Jan 13, 2023
@drewhammond
Copy link

Will this make it into the next 9.3.x release? This is a major annoyance for us too

@leeoniya
Copy link
Contributor

yes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Focus in Dashboards search broken in 9.3.x.
5 participants