-
Notifications
You must be signed in to change notification settings - Fork 221
[docs] update ContextRequestEvent signature #1403
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
Conversation
Document changes to ContextRequestEvent in @lit/context lit/lit#4734
Important The terms of service for this installation has not been accepted. Please ask the Organization owners to visit the Gemini Code Assist Admin Console to sign it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the submission, quick suggestion
|
||
- `readonly contextTarget: Element` | ||
|
||
The original context target of the requester |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Personally not a fan of using context target
to describe contextTarget
. What about something like:
The DOM element that initiated the context request
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will open a PR on @lit/context to update the param description there so this won't get overwritten when the API docs are automatically generated.
Also don't mind the failing CI checks. Pretty sure this happens with forks EDIT: https://pr1403-e495851---lit-dev-5ftespv5na-uc.a.run.app/docs/data/context/#contextrequestevent |
/gcbrun |
Thanks for the quick turnaround! 🚀 |
deployed! |
Document changes to
ContextRequestEvent
in @lit/contextlit/lit#4734
https://github.com/lit/lit/blob/main/packages/context/src/lib/context-request-event.ts#L51-L65