-
-
Notifications
You must be signed in to change notification settings - Fork 32.6k
[material-ui][SpeedDial] Fix navigation with arrow keys when slotProps.fab is defined #46508
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
base: master
Are you sure you want to change the base?
Conversation
Netlify deploy previewhttps://deploy-preview-46508--material-ui.netlify.app/ Bundle size report
|
@@ -424,6 +424,191 @@ describe('<SpeedDial />', () => { | |||
}); | |||
}); | |||
|
|||
describe('dial focus with slotProps.fab', () => { |
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.
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.
Hi! I'm a grad student working on a research project about using large language models to automate code review. Based on your commit cd806bd and the changes in packages/mui-material/src/SpeedDial/SpeedDial.js, my tool generated this comment:
- Since
fabSlotOrigButtonRef
is marked asReact.RefObject?
, ensure that the rest of the code that interacts with this parameter handles it correctly, especially if it is optional. This includes checking for null or undefined values before using it. -
- Since
fabSlotOrigButtonRef
is marked as an optional parameter, there should be a null check to ensure that it is not null before attempting to use it.
- Since
-
- If
fabSlotOrigButtonRef
is intended to be used in the function, update the function's implementation accordingly. If it is not needed, consider removing it from the parameter list.
- If
-
- Ensure that the new parameter
fabSlotOrigButtonRef
is actually used within the function. If it is not used, it may lead to confusion for future developers.
- Ensure that the new parameter
-
- Add unit tests to verify the function's behavior with and without the
fabSlotOrigButtonRef
parameter.
- Add unit tests to verify the function's behavior with and without the
-
- Adjust or add tests to cover changes in behavior due to the new parameter, including edge cases like null or undefined values.
-
- The comment should explain what
fabSlotOrigButtonRef
is used for, similar to how the other parameters are described. This will help future developers understand its role in the function.
- The comment should explain what
-
- Ensure that the newly added parameter
fabSlotOrigButtonRef
follows the same format as the existing parameters.
- Ensure that the newly added parameter
As part of my research, I'm trying to understand how useful these comments are in real-world development. If you have a moment, I'd be super grateful if you could quickly reply to these two yes/no questions:
-
Does this comment provide suggestions from a dimension you hadn’t considered?
-
Do you find this comment helpful?
Thanks a lot for your time and feedback! And sorry again if this message is a bother.
closes #46502
before: https://stackblitz.com/edit/github-kscpcafd?file=src%2FApp.tsx
after: https://stackblitz.com/edit/wrg8wbay?file=src%2FDemo.tsx