-
Notifications
You must be signed in to change notification settings - Fork 22.4k
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
Improve Web/API/Event/Comparison_of_Event_Targets
#33644
base: main
Are you sure you want to change the base?
Improve Web/API/Event/Comparison_of_Event_Targets
#33644
Conversation
Preview URLs External URLs (1)URL:
(comment last updated: 2024-06-12 20:50:00) |
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.
Thank you for this PR, @leon-win !
This is obviously an improvement over what's there now, but I think it has bigger problems, still.
The two main problems, IMO, are:
- it's in the wrong place. Guide pages that live under interface pages don't appear in the site nav, so are basically unfindable.
- I don't think it's worth including the three Firefox-only properties in here. That means there are really just two things to talk about: (1) the difference between
target
andcurrentTarget
, and (2)relatedTarget
. However, sincerelatedTarget
is quite specific to just one event type (and is already documented there: https://developer.mozilla.org/en-US/docs/Web/API/MouseEvent/relatedTarget), it doesn't make sense to talk about it in general event documentation.
So what we should do is:
- document the difference between
target
andcurrentTarget
in https://developer.mozilla.org/en-US/docs/Learn/JavaScript/Building_blocks/Events, which is the main place we talk about events. Probably in its own section after (or maybe just before) the "event delegation" section. - delete this page and redirect to https://developer.mozilla.org/en-US/docs/Learn/JavaScript/Building_blocks/Events
Please let me know if you'd like to have a go at this, or if you'd rather I close this PR and take care of it myself.
Thank you for the review, @wbamberg and I apologize for the long response.
I agree with you.
This is a good solution, but I would like to draw attention to two points:
Therefore, I suggest considering moving the page to a more suitable location instead of deleting it. This will allow to save information about Firefox specific properties (after all, in general it can be useful to someone).
Yes, I will try to do this (but it may not work at a sufficient level, since I am not an English speaking person). And it will take some time =) Therefore, maybe we won’t close this PR, but rather merge it, because it will correct the errors that exist in the current version? |
This is a good point! Then I have a new suggestion: split https://developer.mozilla.org/en-US/docs/Learn/JavaScript/Building_blocks/Events into two pages, by extracting "Event bubbling" and "Event delegation" into a new page, called "Event bubbling", and include the discussion of That would improve the existing learn content, by making the event intro page shorter and giving more space for event bubbling, which is a relatively complex bit of events.
I think
Can it though? I don't know what "f the event was retargeted for some reason other than an anonymous boundary crossing, this will be set to the target before the retargeting occurs." means, and we don't explain. What's anonymous boundary crossing, and when would it be relevant for web developers? We do still have https://developer.mozilla.org/en-US/docs/Web/API/Event/explicitOriginalTarget etc. I'll ask the other maintainers if anyone thinks we should keep this content.
|
This pull request has merge conflicts that must be resolved before it can be merged. |
Description
This PR updates
Web/API/Event/Comparison_of_Event_Targets
. List of changes (in the order of their location in the document):relatedTarget
refers specifically to mouse events,{{Non-standard_Inline}}
labels next to the event target name, similar to this formatting in the sidebar and to relieve the overcrowded third column,EmbedLiveSample
and make its header more general ("Difference between event targets"), because this example is so in its implementation,target
andrelatedTarget
" section,EmbedLiveSample
,Additional details
Some screenshots after changes: