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

[material-ui][Tooltip] Allow auto placement on tooltip #45399

Merged
merged 7 commits into from
Mar 10, 2025
Merged

[material-ui][Tooltip] Allow auto placement on tooltip #45399

merged 7 commits into from
Mar 10, 2025

Conversation

Jtaks
Copy link
Contributor

@Jtaks Jtaks commented Feb 24, 2025

Allow Tooltip to accept Popper's auto placement values.

@mui-bot
Copy link

mui-bot commented Feb 24, 2025 •
edited
Loading

Netlify deploy preview

Bundle size report

No bundle size changes (Toolpad)
No bundle size changes

Generated by 🚫 dangerJS against c554f77

@zannager zannager added the component: tooltip This is the name of the generic UI component, not the React module! label Feb 25, 2025
@zannager zannager requested a review from siriwatknp February 25, 2025 11:32
ZeeshanTamboli
ZeeshanTamboli reviewed Mar 1, 2025
View reviewed changes
Copy link
Member

@ZeeshanTamboli ZeeshanTamboli left a comment

Choose a reason for hiding this comment

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

@Jtaks Could you share a StackBlitz or CodeSandbox example demonstrating auto, auto-start, and auto-end placements in the Tooltip? And also illustrate how they work.

@ZeeshanTamboli ZeeshanTamboli changed the title Allow auto placement on tooltip [Tooltip] Allow auto placement on tooltip Mar 1, 2025
@ZeeshanTamboli ZeeshanTamboli added the package: material-ui Specific to @mui/material label Mar 1, 2025
@Jtaks
Copy link
Contributor Author

Jtaks commented Mar 3, 2025 •
edited
Loading

@ZeeshanTamboli Yes I can. I dont see any information on how to access the codesandboxes for this PR. Could you provide some guidance around that?

@Jtaks
Copy link
Contributor Author

Jtaks commented Mar 3, 2025

@ZeeshanTamboli Never mind, I found it. 👍

@Jtaks
Copy link
Contributor Author

Jtaks commented Mar 3, 2025

https://codesandbox.io/p/sandbox/material-ui-cra-ts-forked-xly385

@ZeeshanTamboli
Copy link
Member

@Jtaks I couldn't see any difference between auto-start and auto-end.

@siriwatknp
Copy link
Member

Thanks for submitting the PR but what issue does this PR will fix? If it's about missing values, can you add a link to that API ref?

@Jtaks
Copy link
Contributor Author

Jtaks commented Mar 7, 2025 •
edited
Loading

@Jtaks I couldn't see any difference between auto-start and auto-end.

@ZeeshanTamboli It seems that may be a bug in the implementation of the Popper component. I am not extending the functionality of Popper, only exposing existing Popper functionality.

what issue does this PR will fix? If it's about missing values, can you add a link to that API ref?

@siriwatknp It is adding missing values. Sorry I could have provided more information. This PR exposes placement values that exist on Popper but are missing in Tooltip.

  • Popper.placement api docs with all values
  • Tooltip.placement api docs missing auto* values

@ZeeshanTamboli
Copy link
Member

ZeeshanTamboli commented Mar 8, 2025 •
edited
Loading

@Jtaks Okay, understood. Thanks. @siriwatknp Can you review?

siriwatknp
siriwatknp approved these changes Mar 10, 2025
View reviewed changes
Copy link
Member

@siriwatknp siriwatknp left a comment

Choose a reason for hiding this comment

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

👍 Good one!

@ZeeshanTamboli ZeeshanTamboli changed the title [Tooltip] Allow auto placement on tooltip [material-ui][Tooltip] Allow auto placement on tooltip Mar 10, 2025
@ZeeshanTamboli ZeeshanTamboli added v6.x enhancement This is not a bug, nor a new feature needs cherry-pick The PR should be cherry-picked to master after merge labels Mar 10, 2025
@ZeeshanTamboli ZeeshanTamboli merged commit f4cc177 into mui:master Mar 10, 2025
25 checks passed
@github-actions GitHub Actions
Copy link

Cherry-pick PRs will be created targeting branches: v6.x

github-actions bot pushed a commit that referenced this pull request Mar 10, 2025
@Jtaks @siriwatknp
@ZeeshanTamboli @web-flow
[material-ui][Tooltip] Allow auto placement on tooltip (#45399)
1d46d45 
Co-authored-by: siriwatknp <siriwatkunaporn@gmail.com>
Co-authored-by: ZeeshanTamboli <zeeshan.tamboli@gmail.com>
@github-actions github-actions bot mentioned this pull request Mar 10, 2025
Merged
mnajdova pushed a commit to mnajdova/material-ui that referenced this pull request Mar 11, 2025
@Jtaks @siriwatknp
@ZeeshanTamboli @mnajdova
[material-ui][Tooltip] Allow auto placement on tooltip (mui#45399)
de44293 
Co-authored-by: siriwatknp <siriwatkunaporn@gmail.com>
Co-authored-by: ZeeshanTamboli <zeeshan.tamboli@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ZeeshanTamboli ZeeshanTamboli ZeeshanTamboli left review comments

@siriwatknp siriwatknp siriwatknp approved these changes

Assignees
No one assigned
Labels
component: tooltip This is the name of the generic UI component, not the React module! enhancement This is not a bug, nor a new feature needs cherry-pick The PR should be cherry-picked to master after merge package: material-ui Specific to @mui/material v6.x
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@Jtaks @mui-bot @ZeeshanTamboli @siriwatknp @zannager