fix(action-sheet, alert, toast): button roles autocomplete with available options - #27940
Conversation
|
|
sean-perkins
left a comment
There was a problem hiding this comment.
Is there an open issue for this problem? If not, can you please create a new issue and link it to this PR?
Thanks!
| export interface AlertButton { | ||
| text: string; | ||
| role?: 'cancel' | 'destructive' | string; | ||
| role?: 'cancel' | 'destructive' | Omit<string, 'cancel' | 'destructive'>; |
There was a problem hiding this comment.
We want to avoid having to re-type the role in multiple places.
Instead, I would consider something similar to what we did for colors to solve a similar problem when using a type union with string:
There was a problem hiding this comment.
Is there a global interface similar to what I mentioned in the Other information section of the initial comment?
If not, I will implement what you have suggested, and continue for now. However, I would suggest there to be something similar, as Ionic is flexible and these interfaces, I believe, are useful.
There was a problem hiding this comment.
👋 currently the interface.d.ts file acts as the global interface. LiteralUnion is currently an internal type, but we could export it to be used for these changes as well: https://github.com/ionic-team/ionic-framework/blob/main/core/src/interface.d.ts#L132
There was a problem hiding this comment.
Awesome, thank you! I'll review and add another member of the team for an additional review as well 👍
I've created an issue and linked it to the initial comment I made to this PR (#27965). |
"x" | "y" | string => LiteralUnion<"x" | "y", string>
sean-perkins
left a comment
There was a problem hiding this comment.
It looks like there are some build errors when compiling the core project:
[ ERROR ] TypeScript: src/components/toast/toast.tsx:372:18
Argument of type 'LiteralUnion<"cancel", string> | undefined' is not
assignable to parameter of type 'string | undefined'.Type
'Pick<string, never> & { _?: undefined; }' is not assignable to type
'string | undefined'.
L371: const role = button.role;
L372: if (isCancel(role)) {
L373: return this.dismiss(undefined, role);
[ ERROR ] TypeScript: src/components/toast/toast.tsx:373:38
Argument of type 'LiteralUnion<"cancel", string> | undefined' is not
assignable to parameter of type 'string | undefined'.
L372: if (isCancel(role)) {
L373: return this.dismiss(undefined, role);
L374: }
Let me know if you have questions around it.
|
My apologies. I was importing |
|
Any updates on this? |
liamdebeasi
left a comment
There was a problem hiding this comment.
Tested on my end with dev build 7.6.7-dev.11706625796.1b3c1ab2, and it works great. Thanks for fixing this!
|
Thank you for contributing! |
Issue number: resolves #27965
https://www.youtube.com/watch?v=a_m7jxrTlaw&list=PLIvujZeVDLMx040-j1W4WFs1BxuTGdI_b&index=3
What is the current behavior?
There is a lack of autocomplete support for AlertButton attribute of
role(there may be similar situations for other components too, however, I was working on this component and found it).What is the new behavior?
cancelanddestructive, and also supports any arbitrary string if passed in.Does this introduce a breaking change?
Other information
I suggest there to be some global interface similar to the following:
I referenced this great video from Matt @mattpocock