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

[pickers] Make the usePickersTranslations hook public #13657

Merged
merged 3 commits into from
Jul 2, 2024

Conversation

flaviendelangle
Copy link
Member

Closes #12691

@mui-bot
Copy link

mui-bot commented Jun 28, 2024

@flaviendelangle flaviendelangle changed the title [pickers] Make useLocaleText public [pickers] Make the useLocaleText hook public Jun 28, 2024
@zannager zannager added the component: pickers This is the name of the generic UI component, not the React module! label Jun 28, 2024
Copy link
Member

@LukasTy LukasTy left a comment

Choose a reason for hiding this comment

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

Nice, thank you for the effort! 👍 👏

docs/data/date-pickers/localization/localization.md Outdated Show resolved Hide resolved
packages/x-date-pickers/src/hooks/useLocaleText.ts Outdated Show resolved Hide resolved
@LukasTy LukasTy added the customization: logic Logic customizability label Jul 1, 2024
@LukasTy
Copy link
Member

LukasTy commented Jul 1, 2024

P.S. In regards to accessing useUtils, it's pretty trivial to do using the current publicly exposed data. 🤷

const useLocalization = () => {
  return React.useContext(MuiPickersAdapterContext);
};
...
const { utils } = useLocalization();

@flaviendelangle
Copy link
Member Author

P.S. In regards to accessing useUtils, it's pretty trivial to do using the current publicly exposed data. 🤷

But MuiPickersAdapterContext is not public right?

@LukasTy
Copy link
Member

LukasTy commented Jul 1, 2024

P.S. In regards to accessing useUtils, it's pretty trivial to do using the current publicly exposed data. 🤷

But MuiPickersAdapterContext is not public right?

Well, it's exported 🤔

{ "name": "MuiPickersAdapterContext", "kind": "Variable" },

@flaviendelangle
Copy link
Member Author

😬 Should not be
It's not documented, I'd be in favor of deprecating it and see if we have request to bring it back (in which case I would rather expose the individual hooks like useUtils rather than the full raw context).

@LukasTy
Copy link
Member

LukasTy commented Jul 2, 2024

😬 Should not be
It's not documented, I'd be in favor of deprecating it and see if we have request to bring it back (in which case I would rather expose the individual hooks like useUtils rather than the full raw context).

I'm all for it. 🙌
Exporting the internal context that is not documented and never mentioned is not good for us and users if anyone is depending on it. 🙈

@LukasTy
Copy link
Member

LukasTy commented Jul 2, 2024

However, it seems that this export has already been added as per the user's request. 🙈
#4367

@flaviendelangle
Copy link
Member Author

Totally forgot this 😆
It was to have parity feature with the lab, but I think we can reconsider with a good migration curve of course.

I'll keep it for a follow up 👍

@LukasTy LukasTy changed the title [pickers] Make the useLocaleText hook public [pickers] Make the usePickersTranslations hook public Jul 2, 2024
Copy link
Member

@LukasTy LukasTy left a comment

Choose a reason for hiding this comment

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

The final result looks great! 💯 💙

One final nitpick: https://github.com/mui/mui-x/issues/12049#issuecomment-1953915199—I've mentioned potentially creating a bit more varied changes to the custom ActionBar slot. 🤔

@flaviendelangle
Copy link
Member Author

We can improve the ActionBar demo in a follow up, it's not strictly related to this change 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: pickers This is the name of the generic UI component, not the React module! customization: logic Logic customizability
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[pickers] Move internal localization hook useLocaleText to Public API
5 participants