-
-
Notifications
You must be signed in to change notification settings - Fork 32.5k
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
[joy-ui][Autocomplete] Add type=button
to clear button
#39263
Conversation
Netlify deploy previewhttps://deploy-preview-39263--material-ui.netlify.app/ Bundle size reportDetails of bundle changes (Toolpad) |
type=button
to clear button
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't really see it fixing the root problem:
- IconButton in Joy UI doesn't set the default type, comparing the two https://codesandbox.io/s/loving-frost-znlnvy?file=/Demo.tsx
import * as React from "react";
import Box from "@mui/joy/Box";
import OpenInNew from "@mui/icons-material/OpenInNew";
import IconButtonJoy from "@mui/joy/IconButton";
import IconButtonMd from "@mui/material/IconButton";
import { CssVarsProvider } from "@mui/joy/styles";
export default function ButtonLink() {
return (
<Box sx={{ display: "flex", gap: 2, alignItems: "center" }}>
<CssVarsProvider>
<IconButtonJoy type={undefined}>
<OpenInNew />
</IconButtonJoy>
</CssVarsProvider>
<IconButtonMd type={undefined}>
<OpenInNew />
</IconButtonMd>
</Box>
);
}
Solved with?
diff --git a/packages/mui-joy/src/IconButton/IconButton.tsx b/packages/mui-joy/src/IconButton/IconButton.tsx
index 8a169ab2d4..e876549638 100644
--- a/packages/mui-joy/src/IconButton/IconButton.tsx
+++ b/packages/mui-joy/src/IconButton/IconButton.tsx
@@ -138,6 +138,7 @@ const IconButton = React.forwardRef(function IconButton(inProps, ref) {
size: sizeProp = 'md',
slots = {},
slotProps = {},
+ type,
...other
} = props;
const buttonGroup = React.useContext(ButtonGroupContext);
- useAutocomplete doesn't set the type="button". Maybe but not sure, it could mess stuff up if developers are using role="button".
diff --git a/packages/mui-base/src/useAutocomplete/useAutocomplete.js b/packages/mui-base/src/useAutocomplete/useAutocomplete.js
index f81827e684..524ebdf1af 100644
--- a/packages/mui-base/src/useAutocomplete/useAutocomplete.js
+++ b/packages/mui-base/src/useAutocomplete/useAutocomplete.js
@@ -1131,10 +1131,12 @@ export function useAutocomplete(props) {
disabled: disabledProp,
}),
getClearProps: () => ({
+ type: 'button',
tabIndex: -1,
onClick: handleClear,
}),
getPopupIndicatorProps: () => ({
+ type: 'button',
tabIndex: -1,
onClick: handlePopupIndicator,
}),
Looking at the implementation, the actual As per your comment, the 2nd one makes most sense as it'll apply the fix to all present and future components using |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 Thanks for the fix!
Closes #39262