Skip to content

[SelectInput] Convert to function component#15410

Merged
oliviertassinari merged 2 commits into
mui:nextfrom
joshwooding:selectinput-function-component
Apr 21, 2019
Merged

[SelectInput] Convert to function component#15410
oliviertassinari merged 2 commits into
mui:nextfrom
joshwooding:selectinput-function-component

Conversation

@joshwooding

@joshwooding joshwooding commented Apr 19, 2019 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Works towards #15231

@joshwooding joshwooding added type: new feature Expand the scope of the product to solve a new problem. scope: select Changes related to the select. labels Apr 19, 2019
@joshwooding joshwooding mentioned this pull request Apr 19, 2019
Closed
29 tasks
@mui-pr-bot

mui-pr-bot commented Apr 19, 2019 •
edited
Loading

Copy link
Copy Markdown

@material-ui/core: parsed: -0.26% 😍, gzip: -0.17% 😍

Details of bundle changes.

Comparing: 3044cb4...b175c50

bundle parsed diff gzip diff prev parsed current parsed prev gzip current gzip
@material-ui/core -0.26% -0.17% 312,618 311,790 84,447 84,305
@material-ui/core/Paper 0.00% -0.02% 67,279 67,279 19,968 19,965
@material-ui/core/Paper.esm 0.00% 0.00% 60,640 60,640 18,884 18,884
@material-ui/core/Popper 0.00% -0.03% 34,907 34,907 11,812 11,809
@material-ui/core/Textarea 0.00% 0.00% 5,327 5,327 2,291 2,291
@material-ui/core/styles/createMuiTheme 0.00% 0.00% 15,898 15,898 5,771 5,771
@material-ui/core/useMediaQuery 0.00% 0.00% 2,106 2,106 974 974
@material-ui/lab 0.00% -0.00% 141,612 141,612 42,722 42,721
@material-ui/styles 0.00% 0.00% 50,833 50,833 15,013 15,013
@material-ui/system 0.00% 0.00% 11,765 11,765 3,928 3,928
Button 0.00% -0.02% 85,621 85,621 25,588 25,584
Modal 0.00% -0.00% 79,901 79,901 24,011 24,010
colorManipulator 0.00% 0.00% 3,904 3,904 1,543 1,543
docs.landing 0.00% 0.00% 50,908 50,908 11,210 11,210
docs.main 0.00% 0.00% 648,518 648,518 201,926 201,926
packages/material-ui/build/umd/material-ui.production.min.js -0.28% -0.26% 293,911 293,076 82,492 82,277

Generated by 🚫 dangerJS against b175c50

oliviertassinari
oliviertassinari reviewed Apr 19, 2019
View reviewed changes

@oliviertassinari oliviertassinari left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Awesome work! I have tried something new with this review: 1. Checkout the pull request. 2. Revert the source back to its original state. 3. Make sure the new tests are green.

Comment thread packages/material-ui/src/Select/SelectInput.test.js Outdated
@joshwooding

joshwooding commented Apr 19, 2019

Copy link
Copy Markdown
Collaborator Author

Awesome work! I have tried something new with this review: 1. Checkout the pull request. 2. Revert the source back to its original state. 3. Make sure the new tests are green.

I try to do this when re-writing the tests :)

@eps1lon

eps1lon commented Apr 20, 2019

Copy link
Copy Markdown
Member

Awesome work! I have tried something new with this review: 1. Checkout the pull request. 2. Revert the source back to its original state. 3. Make sure the new tests are green.

This is something I'll try to do with all my PRs: Tests before implementation. This way it's easier for reviewers to verify: is it a bug? would this test new behavior properly etc. Then you can share different implementations based on the same test plan via git branches. If you do impl after test this becomes very weird (hard reset, rebase etc).

@oliviertassinari
oliviertassinari merged commit 83882fc into mui:next Apr 21, 2019
@joshwooding
joshwooding deleted the selectinput-function-component branch April 21, 2019 12:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@oliviertassinari oliviertassinari oliviertassinari approved these changes

Assignees

No one assigned

Labels

scope: select Changes related to the select. type: new feature Expand the scope of the product to solve a new problem.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@joshwooding @mui-pr-bot @eps1lon @oliviertassinari