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: allow OptionsList to not focus on the search bar #1528

Merged
merged 5 commits into from
Oct 10, 2022

Conversation

chenxzhang
Copy link
Contributor

Original Jira ticket: https://segment.atlassian.net/browse/PROT-4257
Loom: https://www.loom.com/share/79fd27a5ac2a415c93aac856c50c427a

Overview
There's a bug in Protocols tracking plan, where the focus of the input changes to the focus of the filter search bar. This happens because the tracking plan editor pages refreshes every few seconds and the focus is autofocused to the search bar component. This PR adds a prop shouldAutoFocus to OptionsList so that we can disable the autofocus as needed. The change will not affect existing behaviors.

Screenshots (if applicable)

Documentation

  • Updated Typescript types and/or component PropTypes
  • Added / modified component docs
  • Added / modified Storybook stories

@netlify
Copy link

netlify bot commented Oct 7, 2022

Deploy Preview for evergreen-storybook ready!

Name Link
🔨 Latest commit e7e5ee6
🔍 Latest deploy log https://app.netlify.com/sites/evergreen-storybook/deploys/63409c58ffdbd10009a20fd4
😎 Deploy Preview https://deploy-preview-1528--evergreen-storybook.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.

@chenxzhang chenxzhang marked this pull request as ready for review October 7, 2022 20:26
/**
* When true, menu auto focuses on the search/filter bar.
*/
shouldAutoFocus: PropTypes.bool
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we add this to the SelectMenuProps interface in index.d.ts as well? (Listing @default true might be helpful as well)

index.d.ts Outdated
@@ -2387,6 +2387,10 @@ export interface SelectMenuProps extends Omit<PopoverProps, 'position' | 'conten
* When true, show the filter.
*/
hasFilter?: boolean
/**
* @default true. When true, auto focuses on the search/filter bar.
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we move the description on a separate line from the @default JSDoc attribute? I'm thinking that shows up weird in your IDE:

image

vs the expected:

image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i see, thanks for the note!

@chenxzhang chenxzhang merged commit 7b51856 into master Oct 10, 2022
@chenxzhang chenxzhang deleted the chzhang/update-select-menu branch October 10, 2022 14:47
@chenxzhang chenxzhang added the hacktoberfest-accepted Approved PR participating in Hacktoberfest label Oct 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hacktoberfest-accepted Approved PR participating in Hacktoberfest
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants