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

Restrict Toast/Message SeverityType to the only four valid values #2911

Merged
merged 5 commits into from
Jul 1, 2022
Merged

Restrict Toast/Message SeverityType to the only four valid values #2911

merged 5 commits into from
Jul 1, 2022

Conversation

inad9300
Copy link
Contributor

No description provided.

@melloware melloware added the Typescript Issue or pull request is *only* related to TypeScript definition label May 20, 2022
@melloware melloware added this to the 8.2.0 milestone May 20, 2022
@melloware
Copy link
Member

Shouldn't the same be true for MessageSeverityType as well? Also do you have any docs on how this actually does retrict it in TypeScript?

@inad9300
Copy link
Contributor Author

inad9300 commented May 20, 2022

Yes, it seems that it should be the same for MessageSeverityType, I just happened to have found the limitation in toasts while working with them.

As for the information in TypeScript, if you are looking for general information on unions of literal types, this should be a start: https://www.typescriptlang.org/docs/handbook/2/everyday-types.html#literal-types. Though that part isn't new to the code, right? I just removed the extra, odd bit that wasn't letting TypeScript report errors. So I believe a more appropriate question would be, what was the purpose of having (string & {}) in addition to the set of allowed string literals?

@melloware
Copy link
Member

melloware commented May 20, 2022

Yes that was my question how does (string & {}) prevent other types. Thats the part that isn't clear to me.

I haven't seen that in any other Typescript code with unions of literal types.

@inad9300
Copy link
Contributor Author

Wait, but I removed (string & {}). Not sure if we are on the same page here yet 😄

@melloware
Copy link
Member

melloware commented May 20, 2022

HA whoops in my mind you were adding it.... I got the lines backwards :). I totally agree then can you also put the change in for MessageSeverity here too.

@inad9300
Copy link
Contributor Author

Done. I found MessagesSeverityType (plural) too in the process, and I changed it like the others, but it sounds like it should be combined with MessageSeverityType (singular).

@melloware
Copy link
Member

Thanks. I think they keep them separate for now just seems to be a pattern in all the components. The only thing I am thinking is they left the open for people to define their own severity like fatal and then then could just use all the theming styles with fatal to basically make their own severity? I will leave this for Mert to review.

@melloware melloware added the Status: Pending Review Issue or pull request is being reviewed by Core Team label May 21, 2022
@melloware melloware changed the title Restrict ToastSeverityType to the only four valid values Restrict Toast/Message SeverityType to the only four valid values May 23, 2022
@melloware
Copy link
Member

@inad9300 can you fix Badge.d.ts as well it looks like it has the same issue?

@inad9300
Copy link
Contributor Author

inad9300 commented Jun 14, 2022

Good catch! Done.

And Tag.d.ts as well.

@mertsincan
Copy link
Member

Thanks a lot for your contribution, @inad9300 ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Pending Review Issue or pull request is being reviewed by Core Team Typescript Issue or pull request is *only* related to TypeScript definition
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants