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

fix(searchbar): changed event listener to click #386

Merged
merged 5 commits into from
Oct 28, 2024

Conversation

void-hr
Copy link
Contributor

@void-hr void-hr commented Oct 8, 2024

Notes for Reviewers

This PR fixes #373

Signed commits

  • Yes, I signed my commits.

Signed-off-by: Gaurav Shandilya <gsharmaji93@gmail.com>
Copy link

netlify bot commented Oct 8, 2024

Deploy Preview for bejewelled-pegasus-b0ce81 ready!

Name Link
🔨 Latest commit ccea8fa
🔍 Latest deploy log https://app.netlify.com/sites/bejewelled-pegasus-b0ce81/deploys/671f691ffe1753000817f831
😎 Deploy Preview https://deploy-preview-386--bejewelled-pegasus-b0ce81.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 configuration.

@void-hr void-hr marked this pull request as ready for review October 8, 2024 06:33
@void-hr void-hr marked this pull request as draft October 8, 2024 06:34
@void-hr void-hr marked this pull request as ready for review October 8, 2024 09:43
@void-hr void-hr marked this pull request as draft October 9, 2024 19:06
@void-hr void-hr marked this pull request as ready for review October 10, 2024 10:33
@yash37158
Copy link

@void-hr Ensure that the button click event does not cause a focus shift by preventing the default behavior and explicitly managing focus. also for Event bubbling If the flickering is due to event bubbling, make sure to stop the propagation of the event to other elements that might be causing the focus shift.

});
$searchInput.on("keydown", (event) => {
if (event.key === "Enter") {
event.preventDefault();

Choose a reason for hiding this comment

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

Remember to prevent the event from bubbling up and use event.stopPropagation() to prevent the event from bubbling further.

searchBar.focus();
};

searchKey.addEventListener("click", () => {
event.preventDefault();

Choose a reason for hiding this comment

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

Same for the hotkey as well.

@vishalvivekm
Copy link
Member

@void-hr This is hard to review, will you please revert lint changes ?

Let's discuss this during the website call on Monday at 5:30 PM IST (7:00 AM CT).

Please add it as an agenda item to the meeting minutes.

@void-hr void-hr force-pushed the fix/searchbar-click branch 2 times, most recently from b7824fb to 9147495 Compare October 16, 2024 07:59
Signed-off-by: Gaurav Shandilya <gsharmaji93@gmail.com>
@void-hr
Copy link
Contributor Author

void-hr commented Oct 16, 2024

@vishalvivekm, reverted lint changes can you now take a look!

@vishalvivekm
Copy link
Member

Thanks @void-hr
Let's discuss this during the website call today at 5:30 PM IST (7:00 AM CT).

Please add it as an agenda item to the meeting minutes.

@vishalvivekm
Copy link
Member

Thanks @void-hr Let's discuss this during the website call today at 5:30 PM IST (7:00 AM CT).

Please add it as an agenda item to the meeting minutes.

adding

@leecalcote
Copy link
Member

@yash37158 how are we doing here?

@leecalcote
Copy link
Member

Thanks for working on this, @void-hr!

@leecalcote
Copy link
Member

If you haven't joined the Layer5 Community Slack, yet, I invite you to jump in.

@void-hr
Copy link
Contributor Author

void-hr commented Oct 25, 2024

If you haven't joined the Layer5 Community Slack, yet, I invite you to jump in.

Thanks for the invite, however I am there from past 2 weeks I guess 🤔

Copy link

@yash37158 yash37158 left a comment

Choose a reason for hiding this comment

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

LGTM @void-hr

Copy link
Member

@vishalvivekm vishalvivekm left a comment

Choose a reason for hiding this comment

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

Thank much @void-hr.
LGTM 😄

@vishalvivekm vishalvivekm merged commit 38cd2a3 into layer5io:master Oct 28, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

[Docs] Enable Site Search with Button Click
4 participants