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

[AriaNotify] Is NotificationId the right name? #2329

Open
alisonmaher opened this issue Sep 16, 2024 · 3 comments
Open

[AriaNotify] Is NotificationId the right name? #2329

alisonmaher opened this issue Sep 16, 2024 · 3 comments

Comments

@alisonmaher
Copy link

alisonmaher commented Sep 16, 2024

The current design of AriaNotify, as outlined in the spec PR and the explainer, includes a notificationId, which is a non-localized string that can be optionally used by an author to provide an AT with further context about the notification.

We have received feedback that the name notificationId is confusing, as the name implies identification rather than a way to tag common notifications across various web applications, and the name also implies that the ID would potentially need to be unique.

As such, the proposal is to update the name of notificationId to type as suggested in #1958 (comment) to better describe what the property's true intention is.

@MelSumner
Copy link
Contributor

Observe that type is already a pretty overloaded term, could we consider notificationType or something that is more specific?

@alisonmaher
Copy link
Author

@MelSumner I would be open to notificationType. The main question I have is if it would be redundant if used inside ariaNotify(). i.e. Is the 'notification' piece implied in this case? I'm not opposed to a more specific name, like notificationType, though.

@alisonmaher
Copy link
Author

This was discussed at TPAC, with the relevant minutes noted below:

jcraig: this is going to sound like a nitpick, but I think notificationId is the wrong name for it, because it sounds like it needs to be a unique id. But it needs to be a type or a class.

Jamie: for what it's worth, everyone in the last meeting agreed that we should change the name

alisonmaher: I have an issue open to change it to type, we can change it to that

keithamus: I think type is important, because in github regions have surprising semantics sometimes. For this, you call the method for a given string, and that's easy to spot in code review

jcraig: I agree, this is much improved over that

The group seemed to like type as the new name, but as suggested by @MelSumner, I am also open to notificationType.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants