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.

Sign up for GitHub

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

Jump to bottom

fix: searchbar focus on Esc key press #393

Merged
merged 3 commits into from Dec 26, 2022
Merged

fix: searchbar focus on Esc key press #393

merged 3 commits into from Dec 26, 2022

Conversation

shanpriyan
Copy link
Contributor

Prerequisites checklist

What is the purpose of this pull request?

What changes did you make? (Give an overview)

Fixed search bar focusing on pressing Esc

Related Issues

SImilar to eslint/eslint#16699 (for Blog page)

Is there anything you'd like reviewers to focus on?

@netlify
Copy link

netlify bot commented Dec 24, 2022 •
edited

👷 Deploy request for es-eslint pending review.

Visit the deploys page to approve it

Name Link
🔨 Latest commit 51b967d

@netlify
Copy link

netlify bot commented Dec 24, 2022 •
edited

✅ Deploy Preview for new-eslint ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit 51b967d
🔍 Latest deploy log https://app.netlify.com/sites/new-eslint/deploys/63a74c3657648e00086c200c
😎 Deploy Preview https://deploy-preview-393--new-eslint.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@eslint-github-bot eslint-github-bot bot added triage bug Something isn't working labels Dec 24, 2022
@netlify
Copy link

netlify bot commented Dec 24, 2022 •
edited

✅ Deploy Preview for zh-hans-eslint ready!

Name Link
🔨 Latest commit 51b967d
🔍 Latest deploy log https://app.netlify.com/sites/zh-hans-eslint/deploys/63a74c36313c440008e82a8c
😎 Deploy Preview https://deploy-preview-393--zh-hans-eslint.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@netlify
Copy link

netlify bot commented Dec 24, 2022 •
edited

✅ Deploy Preview for ja-eslint ready!

Name Link
🔨 Latest commit 51b967d
🔍 Latest deploy log https://app.netlify.com/sites/ja-eslint/deploys/63a74c3691f15200086b3469
😎 Deploy Preview https://deploy-preview-393--ja-eslint.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@netlify
Copy link

netlify bot commented Dec 24, 2022 •
edited

✅ Deploy Preview for hi-eslint ready!

Name Link
🔨 Latest commit 51b967d
🔍 Latest deploy log https://app.netlify.com/sites/hi-eslint/deploys/63a74c36783030000a6c1b15
😎 Deploy Preview https://deploy-preview-393--hi-eslint.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@netlify
Copy link

netlify bot commented Dec 24, 2022 •
edited

✅ Deploy Preview for fr-eslint ready!

Name Link
🔨 Latest commit 51b967d
🔍 Latest deploy log https://app.netlify.com/sites/fr-eslint/deploys/63a74c36eaf8690008edd8d7
😎 Deploy Preview https://deploy-preview-393--fr-eslint.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@netlify
Copy link

netlify bot commented Dec 24, 2022 •
edited

✅ Deploy Preview for de-eslint ready!

Name Link
🔨 Latest commit 51b967d
🔍 Latest deploy log https://app.netlify.com/sites/de-eslint/deploys/63a74c36550678000861b15c
😎 Deploy Preview https://deploy-preview-393--de-eslint.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@amareshsm amareshsm added accepted and removed triage labels Dec 24, 2022
@netlify
Copy link

netlify bot commented Dec 24, 2022 •
edited

✅ Deploy Preview for pt-br-eslint ready!

Name Link
🔨 Latest commit 51b967d
🔍 Latest deploy log https://app.netlify.com/sites/pt-br-eslint/deploys/63a74c366f652b0008167a68
😎 Deploy Preview https://deploy-preview-393--pt-br-eslint.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

kecrily
kecrily approved these changes Dec 24, 2022
View reviewed changes
Copy link
Member

@kecrily kecrily 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 will merge this PR immediately after the merge at eslint/eslint#16700

harish-sethuraman
harish-sethuraman approved these changes Dec 24, 2022
View reviewed changes
Copy link
Member

@harish-sethuraman harish-sethuraman left a comment

Choose a reason for hiding this comment

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

LGTM, Thanks

@harish-sethuraman
Copy link
Member

Can you fix the lint please? @shanpriyan

@shanpriyan
Copy link
Contributor Author

Can you fix the lint please? @shanpriyan

the CI failed during build
image
Not sure why it's happening, I'm able to build successfully in my local environment

@harish-sethuraman
Copy link
Member

Yea I guess it should be fixed after #392

amareshsm
amareshsm reviewed Dec 24, 2022
View reviewed changes
Copy link
Member

@amareshsm amareshsm left a comment

Choose a reason for hiding this comment

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

eslint/eslint#16699 As mentioned in the issue it doesn't lose focus when we press ESC within the search box. 🤔

@shanpriyan
Copy link
Contributor Author

shanpriyan commented Dec 24, 2022 •
edited

eslint/eslint#16699 As mentioned in the issue it doesn't lose focus when we press ESC within the search box. thinking

Shall I add that too? 🤔 . I thought it was not required as per @harish-sethuraman's comment eslint/eslint#16699 (comment)

@shanpriyan
Copy link
Contributor Author

@amareshsm

1a281ce - Addresses the second point in the issue

@shanpriyan
Copy link
Contributor Author

After 1a281ce

eslint-esc.webm

@amareshsm
Copy link
Member

eslint/eslint#16699 As mentioned in the issue it doesn't lose focus when we press ESC within the search box. thinking

Shall I add that too? 🤔

ya, we can I checked a few sites they have implemented.

https://vercel.com/docs
https://developer.mozilla.org/en-US/

snitin315
snitin315 reviewed Dec 25, 2022
View reviewed changes
Copy link
Contributor

@snitin315 snitin315 left a comment

Choose a reason for hiding this comment

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

Lint job is failing.

amareshsm
amareshsm approved these changes Dec 25, 2022
View reviewed changes
Copy link
Member

@amareshsm amareshsm left a comment

Choose a reason for hiding this comment

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

LGTM

@shanpriyan
Copy link
Contributor Author

shanpriyan commented Dec 25, 2022 •
edited

Lint job is failing.

#393 (comment)

@kecrily kecrily merged commit 6091d76 into eslint:main Dec 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@snitin315 snitin315 snitin315 left review comments

@amareshsm amareshsm amareshsm approved these changes

@harish-sethuraman harish-sethuraman harish-sethuraman approved these changes

@kecrily kecrily kecrily approved these changes

Assignees
No one assigned
Labels
accepted bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@shanpriyan @harish-sethuraman @amareshsm @kecrily @snitin315