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

[Select] Add defaultOpen prop #30212

Merged
merged 13 commits into from Jan 13, 2022
Merged

[Select] Add defaultOpen prop #30212

merged 13 commits into from Jan 13, 2022

Conversation

VicHofs
Copy link
Contributor

@VicHofs VicHofs commented Dec 15, 2021 •
edited by michaldudak

<!-- Thanks so much for your PR, your contribution is appreciated! ❤️ -->

This Pull Request implements the `defaultOpen` prop for the `Select` component, which adds the functionality of toggling the menu automatically on mount.

- [x] I have followed (at least) the [PR section of the contributing guide](https://github.com/mui\-org/material\-ui/blob/HEAD/CONTRIBUTING.md\#sending\-a\-pull\-request\).

Preview: https://deploy-preview-30212--material-ui.netlify.app/components/selects

@mui-pr-bot
Copy link

mui-pr-bot commented Dec 15, 2021 •
edited

Details of bundle changes

Generated by 🚫 dangerJS against 350ce15

@VicHofs VicHofs changed the title implement Select component defaultOpen prop Implement Select component defaultOpen prop Dec 15, 2021
@VicHofs VicHofs mentioned this pull request Dec 15, 2021
Closed
2 tasks
@michaldudak
Copy link
Member

michaldudak commented Dec 15, 2021 •
edited

Thanks for your PR.
There are some issues with the build. Please run yarn prettier, yarn typescript and yarn t Select in your workspace and correct any errors reported.

Additionally, run yarn propTypes and yarn docs:api to generate the documentation files and commit the changes.

michaldudak
michaldudak requested changes Dec 15, 2021
View reviewed changes
docs/src/pages/components/selects/selects-pt.md Outdated Show resolved Hide resolved
packages/mui-material/src/Select/Select.js Outdated Show resolved Hide resolved
docs/src/pages/components/selects/DefaultOpen.tsx Outdated Show resolved Hide resolved
@michaldudak michaldudak added component: select This is the name of the generic UI component, not the React module! new feature New feature or request labels Dec 15, 2021
@michaldudak michaldudak changed the title Implement Select component defaultOpen prop [Select] Add defaultOpen prop Dec 15, 2021
michaldudak
michaldudak reviewed Dec 16, 2021
View reviewed changes
docs/next-env.d.ts Outdated Show resolved Hide resolved
docs/src/pages/components/selects/DefaultOpen.tsx Outdated Show resolved Hide resolved
docs/src/pages/components/selects/selects-pt.md Outdated Show resolved Hide resolved
packages/mui-material/src/Select/Select.d.ts Outdated Show resolved Hide resolved
packages/mui-material/src/Select/Select.js Outdated Show resolved Hide resolved
packages/mui-material/src/Select/SelectInput.js Outdated Show resolved Hide resolved
packages/mui-material/src/Select/SelectInput.js Outdated Show resolved Hide resolved
VicHofs
VicHofs commented Dec 16, 2021
View reviewed changes
Copy link
Contributor Author

@VicHofs VicHofs left a comment

Choose a reason for hiding this comment

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

Review observations addressed.

docs/src/pages/components/selects/DefaultOpen.tsx Outdated Show resolved Hide resolved
packages/mui-material/src/Select/SelectInput.js Outdated Show resolved Hide resolved
packages/mui-material/src/Select/SelectInput.js Outdated Show resolved Hide resolved
michaldudak
michaldudak reviewed Dec 30, 2021
View reviewed changes
Copy link
Member

@michaldudak michaldudak left a comment

Choose a reason for hiding this comment

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

Sorry for the delayed response. One more thing that's missing here is a unit test. Could you please add one to verify if the added function works correctly?

docs/src/pages/components/selects/DefaultOpen.tsx Outdated Show resolved Hide resolved
@VicHofs
Copy link
Contributor Author

VicHofs commented Jan 4, 2022

Removed the unused demos and added a unit test for the new defaultOpen prop.

Sorry for the delay on my part too. Happy new year!

VicHofs
VicHofs commented Jan 4, 2022
View reviewed changes
Copy link
Contributor Author

@VicHofs VicHofs left a comment

Choose a reason for hiding this comment

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

Requested review changes addressed.

@VicHofs @michaldudak
Update unit test code to use available helper functions
28eb68a 
Co-authored-by: Michał Dudak <michal.dudak@gmail.com>
michaldudak
michaldudak reviewed Jan 5, 2022
View reviewed changes
Copy link
Member

@michaldudak michaldudak left a comment •
edited

Choose a reason for hiding this comment

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

Almost there :) One more thing to correct to make the tests pass.
Also, please merge in the latest master branch.

packages/mui-material/src/Select/Select.test.js Outdated Show resolved Hide resolved
@VicHofs @michaldudak
Update packages/mui-material/src/Select/Select.test.js
5ebbcd5 
Implement workaround for `aria-hidden` inhibiting `getByRole`

Co-authored-by: Michał Dudak <michal.dudak@gmail.com>
@mui-bot
Copy link

mui-bot commented Jan 13, 2022 •
edited

Details of bundle changes

Generated by 🚫 dangerJS against ad1403c

VicHofs
VicHofs commented Jan 13, 2022
View reviewed changes
Copy link
Contributor Author

@VicHofs VicHofs left a comment

Choose a reason for hiding this comment

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

Requested changes applied.

michaldudak
michaldudak approved these changes Jan 13, 2022
View reviewed changes
Copy link
Member

@michaldudak michaldudak left a comment

Choose a reason for hiding this comment

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

All good! Thanks a lot for your work!

@michaldudak michaldudak merged commit 01bd5a0 into mui:master Jan 13, 2022
@VicHofs VicHofs deleted the feature/select-default-open branch January 13, 2022 14:09
@VicHofs
Copy link
Contributor Author

VicHofs commented Jan 13, 2022

Awesome, thank you!

wladimirguerra pushed a commit to wladimirguerra/material-ui that referenced this pull request Feb 2, 2022
@VicHofs @michaldudak @wladimirguerra
[Select] Add defaultOpen prop (mui#30212)
e42a824 
Co-authored-by: Michał Dudak <michal@dudak.me>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@michaldudak michaldudak michaldudak approved these changes

Assignees
No one assigned
Labels
component: select This is the name of the generic UI component, not the React module! new feature New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@VicHofs @mui-pr-bot @michaldudak @mui-bot