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] How can I get access to the Pickers Adapter Utils for use with usePickersTranslations? #13768

Closed
kealjones-wk opened this issue Jul 8, 2024 · 18 comments · Fixed by #14505
Assignees
Labels
component: pickers This is the name of the generic UI component, not the React module! support: commercial Support request from paid users support: premium standard Support request from a Premium standard plan user. https://mui.com/legal/technical-support-sla/

Comments

@kealjones-wk
Copy link
Contributor

kealjones-wk commented Jul 8, 2024

The problem in depth

#13657 Added usePickersTranslations which is awesome, but it didn't provide a public version of useUtils like was requested in #12691

Access to the Adapter Utils is necessary to use at least 3 of the properties on LocaleText, see: openDatePickerDialogue, openTimePickerDialogue, and clockLabelText. There is currently no public way to get the utils for this argument that i know of. Is there some way that i don't know of?

Actual in depth issue explained in a comment below: #13768 (comment)

Your environment

No response

Search keywords: useUtils, date pickers, localization, localeText, adapter, utils
Order ID: 82849

@kealjones-wk kealjones-wk added status: waiting for maintainer These issues haven't been looked at yet by a maintainer support: commercial Support request from paid users labels Jul 8, 2024
@LukasTy
Copy link
Member

LukasTy commented Jul 9, 2024

Hello @kealjones-wk.
Thank you for rushing to use the new export and creating this issue. 🙏
Could you tell me a bit more about your use case?
Where do you need to use these mentioned localization functions?
Are you building a custom Text Field? Or a Clock? 🤔

@LukasTy LukasTy added status: waiting for author Issue with insufficient information component: pickers This is the name of the generic UI component, not the React module! and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Jul 9, 2024
@LukasTy LukasTy changed the title [question] [pickers] How can I get access to the Pickers Adapter Utils for use with usePickersTranslations? [pickers] How can I get access to the Pickers Adapter Utils for use with usePickersTranslations? Jul 9, 2024
@michelengelen michelengelen added the support: premium standard Support request from a Premium standard plan user. https://mui.com/legal/technical-support-sla/ label Jul 15, 2024
Copy link

The issue has been inactive for 7 days and has been automatically closed.

@kealjones-wk
Copy link
Contributor Author

kealjones-wk commented Jul 24, 2024

Hello @LukasTy sorry for the delay in response I went on holiday after creating this and just got back.

Our use case is Wrapping DateRangePicker (and probably others in the future) to change its functionality to act and look more similar to the DatePicker component, meaning having the Open Picker Button/Icon Adornment instead of opening on click, adding ability to use the keyboard with the calendar, etc.

Because we added the Open Picker Button we need openDatePickerDialogue to set the aria label on that button to match DatePicker's accessibility. I can go into this more if you'd like but that is the tldr.

@github-actions github-actions bot added status: waiting for maintainer These issues haven't been looked at yet by a maintainer and removed status: waiting for author Issue with insufficient information labels Jul 24, 2024
@github-actions github-actions bot reopened this Jul 24, 2024
@flaviendelangle
Copy link
Member

I struggle to understand what is missing for you.
You should have access to openDatePickerDialogue in usePickersTranslations.

If you could create a working reproduction case, it would help us a lot 🙏

@flaviendelangle flaviendelangle added status: waiting for author Issue with insufficient information and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Jul 29, 2024
@kealjones-wk
Copy link
Contributor Author

kealjones-wk commented Jul 31, 2024

Note

Edited emphasises and word choices to not be such a doo doo head.

You should have access to openDatePickerDialogue in usePickersTranslations.

@flaviendelangle Yes, I have access to the methods, but thats not the problem. The problem is that they are unusable because they require a utils/adapter argument and without useUtils being public there is no way to provide that argument to those localeText methods.

I cannot create a working reproduction... but here is an example of not being able to use the method, see line 25 of Demo.tsx: https://stackblitz.com/edit/react-qyafei-7h6nfa?file=Demo.tsx

For some further context:
openDatePickerDialogue requires a utils arg: https://github.com/mui/mui-x/blob/master/packages/x-date-pickers/src/locales/utils/pickersLocaleTextApi.ts#L73
openTimePickerDialogue requires a utils arg: https://github.com/mui/mui-x/blob/master/packages/x-date-pickers/src/locales/utils/pickersLocaleTextApi.ts#L74
clockLabelText requires an adapter arg: https://github.com/mui/mui-x/blob/master/packages/x-date-pickers/src/locales/utils/pickersLocaleTextApi.ts#L64

Now check:

getOpenDialogAriaText:
props.localeText?.openDatePickerDialogue ?? translations.openDatePickerDialogue,

and follow that through to the useDesktopPicker hook here and notice that it is getting passed a utils var:

'aria-label': getOpenDialogAriaText(pickerFieldProps.value, utils),

that utils var is set using useUtils which is internal and not publicly available:

is there some other way to get the adapter/utils that is public? cause if not openDatePickerDialogue, openTimePickerDialogue, and clockLabelText are unusable without useUtils being a public hook.

Let me know if you need anything else. :)

@github-actions github-actions bot added status: waiting for maintainer These issues haven't been looked at yet by a maintainer and removed status: waiting for author Issue with insufficient information labels Jul 31, 2024
@flaviendelangle
Copy link
Member

OK, I totally missed this "detail".

@LukasTy, we should be able to host the utils at a higher level instead of passing it when calling the translation function.
Or we could simply export useUtils

@michelengelen
Copy link
Member

If I get it right the utils are actually just the current adapter.
IMO it would not hurt to just export useUtils. 🤷🏼

@flaviendelangle
Copy link
Member

flaviendelangle commented Aug 1, 2024

I like the idea that the user never has to directly interact with the adapter.
He gives an adapter so that the pickers works perfectly with his date library, but he always interacts with his date using his date library not its adapter.

Right now the adapter is public API, so I respect semver, but people never have to use it so I have no bad feeling breaking things in majors.
If we export useUtils, my "fear" is that people start using adapters, start calling its methods because it can be convenient and that we loose this flexibility.

So I'm not a big fan of exporting useUtils if we have another solution.

Plus, if utils is an internal thing, then having to do:

const utils = useUtils();
const translations = usePickersTranslations();
const label = translations.openTimePickerDialogue(value, utils);

Is a worse DX than this IMHO:

const translations = usePickersTranslations();
const label = translations.openTimePickerDialogue(value);

Since we ask people to import something from our package just to pass it back.

@michelengelen
Copy link
Member

Good points @flaviendelangle ... I'll look into moving the utils a level up so that they don't have to be passed into these methods

@michelengelen michelengelen self-assigned this Aug 2, 2024
@michelengelen michelengelen removed the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Aug 2, 2024
@LukasTy
Copy link
Member

LukasTy commented Aug 2, 2024

@flaviendelangle That is a great suggestion. 👍
I can only second the argument to avoid publically exposing adapter utils if possible unless there is a very clear need.

In this case, I fear that the proposed change is hardly doable without a BC, at least a slight one, because translations are public and if we decide to change their API then it could mean a migration to someone with custom local translations.
Although, as we can see, these translations are hardly used in a custom way, given that it is not easy to change the built-in behavior... 🙈

As for the technical part of the change, we are talking about something like this?

-openDatePickerDialogue: (value, utils) =>
+openDatePickerDialogue: (value, formattedDate) =>
-    value !== null && utils.isValid(value)
+    value !== null && formattedDate
-      ? `Choose date, selected date is ${utils.format(value, 'fullDate')}`
+      ? `Choose date, selected date is ${formattedDate}`
      : 'Choose date',

And on the usage side:

-'aria-label': getOpenDialogAriaText(pickerFieldProps.value, utils),
+'aria-label': getOpenDialogAriaText(pickerFieldProps.value, utils.isValid(pickerFieldProps.value) ? utils.format(pickerFieldProps.value, 'fullDate') : null),

@flaviendelangle
Copy link
Member

What if we keep the utils for now but make it nullable, and stop using it in the actual translations.
That way we don't have a BC in the signature of the translations and we can remove the old param in v8.

As for the technical part, I did not dive into it honestly.
My first idea was to change the signature of the translation object to be something like (utils) => PickersLocaleText but this is probably not doable without a BC.

@kealjones-wk
Copy link
Contributor Author

kealjones-wk commented Aug 3, 2024

I am 100% behind @flaviendelangle idea of making the 2nd arg nullable/undefinable and mui using the utils internal instead of requiring the arg. That seems like a clean and easy change without the need for a BC but opens the 2nd arg for removal in like v6/7 if desired.

PERSONALLY, I don't think there are many consumers of this API besides me as no one tends to be complaining about this besides me, AND the fact that the ONLY way to even provide that 2nd arg that i can think of is to construct your own unique instance of an adapter that might not match the one that was provided by the consumers which would cause its own issues. so like that utils/adapter arg is impossible to use currently anyway so its not gonna hurt when its removed in the future.

@kealjones-wk
Copy link
Contributor Author

PS. @flaviendelangle sorry for any rudeness in my comment above. I was in a bad headspace that week. I should have explained much of those details in the original issue. Thank you for putting up with me anyway and sorry about that. <3

@flaviendelangle
Copy link
Member

No offense taken 🙏

@kealjones-wk
Copy link
Contributor Author

Just wanted to check in on this and see if it was gonna be picked up?

@flaviendelangle
Copy link
Member

@kealjones-wk I took over the topic
We should be able to have it delivered in the coming weeks.

@kealjones-wk
Copy link
Contributor Author

<3 awesome! Thanks so much @flaviendelangle

Copy link

github-actions bot commented Sep 6, 2024

This issue has been closed. If you have a similar problem but not exactly the same, please open a new issue.
Now, if you have additional information related to this issue or things that could help future readers, feel free to leave a comment.

Note

We value your feedback @kealjones-wk! How was your experience with our support team?
If you could spare a moment, we'd love to hear your thoughts in this brief Support Satisfaction survey. Your insights help us improve!

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! support: commercial Support request from paid users support: premium standard Support request from a Premium standard plan user. https://mui.com/legal/technical-support-sla/
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants