[pickers] Add optional id attribute on shortcut items

[noraleonte]

• I have followed (at least) the PR section of the contributing guide.

closes #12968

[mui-bot]

Deploy preview: https://deploy-preview-12976--material-ui-x.netlify.app/

Generated by 🚫 dangerJS against b89a0b3

[michelengelen]

LGTM

[LukasTy]

Sorry, a rushed request for changes. Will leave full feedback soon. 😉
Short version: it doesn't work and creates react errors

[michelengelen]

Sorry, a rushed request for changes. Will leave full feedback soon. 😉 Short version: it doesn't work and creates react errors

Why is that? I tried it last time and just now again and it did not throw any errors for me.

[LukasTy]

WDYT about the following proposal:

diff --git a/packages/x-date-pickers/src/PickersShortcuts/PickersShortcuts.tsx b/packages/x-date-pickers/src/PickersShortcuts/PickersShortcuts.tsx
index 8e36ecf55..09baa499e 100644
--- a/packages/x-date-pickers/src/PickersShortcuts/PickersShortcuts.tsx
+++ b/packages/x-date-pickers/src/PickersShortcuts/PickersShortcuts.tsx
@@ -12,7 +12,11 @@ interface PickersShortcutsItemGetValueParams<TValue> {
 export interface PickersShortcutsItem<TValue> {
   label: string;
   getValue: (params: PickersShortcutsItemGetValueParams<TValue>) => TValue;
-  key?: string;
+  /**
+   * Identifier of the shortcut.
+   * If provided, it will be used as the key of the shortcut.
+   */
+  id?: string;
 }
 
 export type PickersShortcutsItemContext = Omit<PickersShortcutsItem<unknown>, 'getValue'>;
@@ -65,7 +69,7 @@ function PickersShortcuts<TValue>(props: PickersShortcutsProps<TValue>) {
     const newValue = getValue({ isValid });
 
     return {
-      key: item?.key || item.label,
+      ...item,
       label: item.label,
       onClick: () => {
         onChange(newValue, changeImportance, item);
@@ -89,7 +93,7 @@ function PickersShortcuts<TValue>(props: PickersShortcutsProps<TValue>) {
     >
       {resolvedItems.map((item) => {
         return (
-          <ListItem key={item.label}>
+          <ListItem key={item.id ?? item.label}>
             <Chip {...item} />
           </ListItem>
         );

I'm not a fan of exposing a prop that is directly targetted towards just being a React key—an implementation detail.
If we call the field id, then it can be used by users to identify the shortcut item that has been clicked and act accordingly.
Currently, they only receive label, which is a fragile piece of information to take for granted as unique. 🙈
Like, even if it's not currently typed, users could pass color: 'primary' and make such shortcut chip seem selected.

[LukasTy]

Why is that? I tried it last time and just now again and it did not throw any errors for me.

It doesn't do anything, because the key on Listitem is still left as is.
The error is thrown on CodeSandbox, locally I didn't see it as well. 🙈

[noraleonte]

@LukasTy What you are proposing is great, I think the main pain point here was that users couldn't give unique identifiers. id should solve that 👌

Like, even if it's not currently typed, users could pass color: 'primary' and make such shortcut chip seem selected.

Slightly unrelated, but circling back to this, we could maybe type it 🤷‍♀️ Technically, a shortcut item should accept all the props that a chip does

[LukasTy]

The solution look great! 👏

Slightly unrelated, but circling back to this, we could maybe type it 🤷‍♀️ Technically, a shortcut item should accept all the props that a chip does

Yes, that's a great observation.
By doing that we could potentially provide a solution for #10120 as a recipe because I'm not sure if we should be providing that behavior OOB as it is probably impossible without additional performance impact that everybody might not desire. 🙈