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

[InputBase] Add prop for disabling global styles #29213

Merged
merged 7 commits into from
Dec 13, 2021
Merged

[InputBase] Add prop for disabling global styles #29213

merged 7 commits into from
Dec 13, 2021

Conversation

bryan-hunter
Copy link
Contributor

@bryan-hunter bryan-hunter commented Oct 21, 2021 •
edited

Related to this discussion: #28070 (comment)

Problem:
InputBase injects / removes a global style into <head> on every mount / dismount to help with an auto-filling issue. This can cause initial performance rendering issues if you are loading a lot of Input components at once.

<GlobalStyles
    styles={{
      '@keyframes mui-auto-fill': { from: { display: 'block' } },
      '@keyframes mui-auto-fill-cancel': { from: { display: 'block' } },
    }}
  />

Solution:

This PR introduces an optional disableInjectingGlobalStyles to the InputBase component that will disable this from happening. Developers that want to leverage this, will likely want to inject these styles at the root on their app once.

@mnajdova

@mui-pr-bot
Copy link

mui-pr-bot commented Oct 21, 2021 •
edited

Details of bundle changes

@material-ui/core: parsed: +Infinity% , gzip: +Infinity%
@material-ui/lab: parsed: +Infinity% , gzip: +Infinity%
@material-ui/styles: parsed: +Infinity% , gzip: +Infinity%
@material-ui/private-theming: parsed: +Infinity% , gzip: +Infinity%
@material-ui/system: parsed: +Infinity% , gzip: +Infinity%
@material-ui/unstyled: parsed: +Infinity% , gzip: +Infinity%
@material-ui/utils: parsed: +Infinity% , gzip: +Infinity%
@mui/material-next: parsed: +Infinity% , gzip: +Infinity%
@mui/joy: parsed: +Infinity% , gzip: +Infinity%

Generated by 🚫 dangerJS against 16ad8e2

mnajdova
mnajdova requested changes Oct 25, 2021
View reviewed changes
Copy link
Member

@mnajdova mnajdova left a comment

Choose a reason for hiding this comment

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

Let's give some more context on why the property should be used. Otherwise looks good 👌

packages/mui-material/src/InputBase/InputBase.d.ts Outdated Show resolved Hide resolved
@bryan-hunter
Copy link
Contributor Author

@mnajdova checking back in on this :) no rush

@bryan-hunter
Copy link
Contributor Author

@mnajdova bumping just in case it got lost

@mbrookes mbrookes added new feature New feature or request component: text field This is the name of the generic UI component, not the React module! labels Nov 15, 2021
@bryan-hunter
Copy link
Contributor Author

bump again

@bryan-hunter
Copy link
Contributor Author

@mnajdova bump again

@bryan-hunter
Copy link
Contributor Author

@mnajdova @mbrookes bump :P

@mbrookes
Copy link
Member

@bryan-hunter So sorry, it seems we need to figure out how to better manage code review. Your PR shouldn't have waited this long without some sort of feedback, even if to be rejected (to save you spending time on revisions).

I've asked the responsible engineering manager to step in.

@bryan-hunter
Copy link
Contributor Author

@bryan-hunter So sorry, it seems we need to figure out how to better manage code review. Your PR shouldn't have waited this long without some sort of feedback, even if to be rejected (to save you spending time on revisions).

I've asked the responsible engineering manager to step in.

No worries, and thank you!

mnajdova
mnajdova approved these changes Dec 13, 2021
View reviewed changes
Copy link
Member

@mnajdova mnajdova 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 delay, I was flooded with notifications. It's a great first contribution @bryan-hunter 👌

@mnajdova mnajdova changed the title [InputBase] New property for disabling inject of GlobalStyles [InputBase] Add prop for disabling global styles Dec 13, 2021
@mnajdova mnajdova merged commit e36634c into mui:master Dec 13, 2021
@mnajdova mnajdova mentioned this pull request Mar 2, 2022
Closed
2 tasks
@manuel-woelker
Copy link

manuel-woelker commented Mar 3, 2022 •
edited

Just a quick note for other people getting here and having performance issues with many input fields, here is how I solved it:

const theme = createTheme({
	components: {
		MuiInputBase: {
			defaultProps: {
				// Needed to prevent adding a global style for every input field
				disableInjectingGlobalStyles: true,
			},
		},
	},
});

ReactDOM.render(
	<React.StrictMode>
		<ThemeProvider theme={theme}>
			<App />
		</ThemeProvider>
	</React.StrictMode>,
	document.getElementById("root"),
);

I think the global style injection should be memoized or something, as this is an easy trap to fall into and leaves a bad impression of the otherwise great performance of MUI...

@netgfx
Copy link

netgfx commented Jan 12, 2023

This causes Cannot read properties of undefined (reading 'soft') when a component is loaded

@mnajdova mnajdova mentioned this pull request May 10, 2023
Closed
2 tasks
This was referenced Oct 27, 2023
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mnajdova mnajdova mnajdova approved these changes

Assignees

@mnajdova mnajdova

Labels
component: text field 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
6 participants
@bryan-hunter @mui-pr-bot @mbrookes @manuel-woelker @netgfx @mnajdova