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.

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

[material-ui][SpeedDialAction] Props id does not appear in HTML #36220

Open
2 tasks done
MatetlotDev opened this issue Feb 16, 2023 · 7 comments
Open
2 tasks done

[material-ui][SpeedDialAction] Props id does not appear in HTML #36220

MatetlotDev opened this issue Feb 16, 2023 · 7 comments
Assignees
Labels
component: speed dial This is the name of the generic UI component, not the React module! package: material-ui Specific to @mui/material status: expected behavior Does not imply the behavior is intended. Just that we know about it and can't work around it

Comments

@MatetlotDev
Copy link

Duplicates

  • I have searched the existing issues

Latest version

  • I have tested the latest version

Steps to reproduce 🕹

Link to live example: https://codesandbox.io/s/proud-water-fyxu9l?file=/src/App.js

Reproduce

  1. Launch the sandbox
  2. Open up the inspector
  3. Try to search The Ids of the SpeedDialAction components

Actual result

The IDs are not present in the html button components.

Expected result

See the IDs of the actions on the button components.

Current behavior 😯

We do not see the Ids on the button elements

Expected behavior 🤔

We should see the Ids

Context 🔦

I want to add some IDs on the actions so I can make some tests and query the actions by their IDs

Your environment 🌎

npx @mui/envinfo
  Don't forget to mention which browser you used.
  Output from `npx @mui/envinfo` goes here.
@MatetlotDev MatetlotDev added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Feb 16, 2023
@zannager zannager added the component: speed dial This is the name of the generic UI component, not the React module! label Feb 16, 2023
@michaldudak
Copy link
Member

Thanks for the report. The SpeedDialAction follows the rules we have for forwarding extra props - it spreads them on the root element. However, in this case, the root element is a tooltip, and having these extra props on it is confusing and likely doesn't make sense.

We can't change how it works currently without introducing a breaking change, but fortunately, there is a workaround you can use: the FabProps prop.

{actions.map((action) => (
  <SpeedDialAction
    key={action.name}
    icon={action.icon}
    tooltipTitle={action.name}
    FabProps={{ id: action.id }}
  />
))}

It sets its props on the button itself, so this should help you target the button.

I'll add this to the v6 milestone, so we can consider redesigning the component in the next major version.

@michaldudak michaldudak added status: expected behavior Does not imply the behavior is intended. Just that we know about it and can't work around it and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Feb 16, 2023
@michaldudak michaldudak removed their assignment Feb 16, 2023
@michaldudak michaldudak added this to the v6 milestone Feb 16, 2023
@MatetlotDev
Copy link
Author

Alright, I understand now why I couldn't see my IDs, thank you very much for your workaround, It does work perfectly indeed !

@michaldudak
Copy link
Member

Let's leave the issue open, so we remember about this problem when designing the next version.

@michaldudak michaldudak reopened this Feb 16, 2023
@Wire1lovet

This comment was marked as off-topic.

@danilo-leal danilo-leal moved this to Backlog in Material UI Dec 1, 2023
@DiegoAndai DiegoAndai moved this from Backlog to Selected in Material UI Feb 19, 2024
@danilo-leal danilo-leal added the package: material-ui Specific to @mui/material label Mar 5, 2024
@danilo-leal danilo-leal changed the title [SpeedDialAction] props id set to SpeedDialAction does not appear in html [material-ui][SpeedDialAction] Props id does not appear in HTML Mar 5, 2024
@mnajdova mnajdova assigned aarongarciah and unassigned mnajdova Apr 4, 2024
@aarongarciah
Copy link
Member

I see pros and cons with following the rule of forwarding extra props on the root element in SpeedDialAction. While it's consistent with every other component, it's counterintuitive for devs who don't know how it's implemented (the root element is a Tooltip). At the same time, we have FabProps today, so the issue can be circumvented. Note that the plan to deprecate FabProps is on halt (see #41281).

@mnajdova @DiegoAndai, do you know if we have other components that break this rule? Or are there other components that have the same "problem" where the root element is not what it seems to be?

@aarongarciah
Copy link
Member

After some discussions with the team, we decided to change the approach and we'll be forwarding props to the SpeedDialAction button instead of the tooltip. This is what devs expect. We'll also add a slot for the tooltip in case someone needs to customize the tooltip. We'll also take a look to other components to see if some of them would benefit from such a change.

@mnajdova
Copy link
Member

We can make the change in stages.

  1. Add slots/slotProps where the root would be the Fab and the tooltip will be a different slot
  2. Update the docs to teach people to use this API
  3. Deprecate the FabProps
  4. In the next major, make the root props being spread on the Fab.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: speed dial This is the name of the generic UI component, not the React module! package: material-ui Specific to @mui/material status: expected behavior Does not imply the behavior is intended. Just that we know about it and can't work around it
Projects
Status: In progress
Development

Successfully merging a pull request may close this issue.

7 participants