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

Add event when highlight event listener is registered #27

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

runn-vermel
Copy link

@runn-vermel runn-vermel commented Oct 13, 2016

Fix #26

@arthurevans

If you place prism highlighter in another component, there's no way to know when the highlight event listener is registered, resulting in a possible race condition - highlight events being sent out before the event listener is even registered.

By firing a (prism-highlighter-highlight-event-registered) event after the listener is registered, the host can listen for the fired event, and act accordingly.

runn-vermel and others added 7 commits October 6, 2016 09:33
If you place prism highlighter in another component, there's no way to know when the highlight event listener is registered, resulting in a possible race condition - highlight events being sent out before the event listener is even registered.

By firing a (prism-highlighter-highlight-event-registered) event after the listener is registered, the host can listen for the fired event, and act accordingly.
added @event comment so the event shows up in the auto generated docs.
Added references to how to make shadow dom work.
Firing event when highlight event listener is registered

If you place prism highlighter in another component, there's no way to know when the highlight event listener is registered, resulting in a possible race condition - highlight events being sent out before the event listener is even registered.

By firing a (prism-highlighter-highlight-event-registered) event after the listener is registered, the host can listen for the fired event, and act accordingly.

I've also improved shadow dom documentation, and added references to how to make shadow dom work.
@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.


  • If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
  • If you signed the CLA as a corporation, please let us know the company's name.

@runn-vermel
Copy link
Author

Hi guys,

I signed it!

Thanks,
Runn Vermel
Senior Design Technologist
GE Design & Experience / UX CoE
http://www.gesoftware.com

T +1 925 242 6831
[email protected]
GE imagination at work

From: googlebot [email protected]
Reply-To: PolymerElements/prism-element [email protected]
Date: Thursday, October 13, 2016 at 2:42 PM
To: PolymerElements/prism-element [email protected]
Cc: "Vermel, Runn (GE Digital)" [email protected], Author [email protected]
Subject: EXT: Re: [PolymerElements/prism-element] Add event when highlight event listener is registered (#27)

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/https://urldefense.proofpoint.com/v2/url?u=https-3A__cla.developers.google.com_&d=DQMFaQ&c=IV_clAzoPDE253xZdHuilRgztyh_RiV3wUrLrDQYWSI&r=kjMxTObkzmL5Icm-bloWj-5PiLOC1GCXcnZ3DyUZgbc&m=J_Z2mS6NM3wLn9oXb6ouJONYrN1rajiYTs8R7KDNaBc&s=4vyhpM73eYEEMSZ6_to2Jqht8JqmibZqyGYy7bZqr7I&e= to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.


  • If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA datahttps://urldefense.proofpoint.com/v2/url?u=https-3A__cla.developers.google.com_clas&d=DQMFaQ&c=IV_clAzoPDE253xZdHuilRgztyh_RiV3wUrLrDQYWSI&r=kjMxTObkzmL5Icm-bloWj-5PiLOC1GCXcnZ3DyUZgbc&m=J_Z2mS6NM3wLn9oXb6ouJONYrN1rajiYTs8R7KDNaBc&s=JgzOVcTn2-q4Bdijl5MqXbbIddd_u2wz7zd67vmkX2o&e= and verify that your email is set on your git commitshttps://urldefense.proofpoint.com/v2/url?u=https-3A__help.github.com_articles_setting-2Dyour-2Demail-2Din-2Dgit_&d=DQMFaQ&c=IV_clAzoPDE253xZdHuilRgztyh_RiV3wUrLrDQYWSI&r=kjMxTObkzmL5Icm-bloWj-5PiLOC1GCXcnZ3DyUZgbc&m=J_Z2mS6NM3wLn9oXb6ouJONYrN1rajiYTs8R7KDNaBc&s=NaIe7Pn45N_CwwgylMiPafSr_-e6PImPQbY9kTYQrdI&e=.
  • If you signed the CLA as a corporation, please let us know the company's name.


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHubhttps://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_PolymerElements_prism-2Delement_pull_27-23issuecomment-2D253647451&d=DQMFaQ&c=IV_clAzoPDE253xZdHuilRgztyh_RiV3wUrLrDQYWSI&r=kjMxTObkzmL5Icm-bloWj-5PiLOC1GCXcnZ3DyUZgbc&m=J_Z2mS6NM3wLn9oXb6ouJONYrN1rajiYTs8R7KDNaBc&s=kSB0NifYSJMXQLCj7ZVMvBgvD58IdygMStR4Aa5QkEc&e=, or mute the threadhttps://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_notifications_unsubscribe-2Dauth_AEGM8b6ciOLDsmbvyi5KgKz9BBWvmnjsks5qzqXOgaJpZM4KWaCU&d=DQMFaQ&c=IV_clAzoPDE253xZdHuilRgztyh_RiV3wUrLrDQYWSI&r=kjMxTObkzmL5Icm-bloWj-5PiLOC1GCXcnZ3DyUZgbc&m=J_Z2mS6NM3wLn9oXb6ouJONYrN1rajiYTs8R7KDNaBc&s=3LaC2DYHYqb_Tn2-1DA7dA88sGvneM9VdOXDURAlZm4&e=.

@googlebot
Copy link

CLAs look good, thanks!

@FredKSchott
Copy link
Contributor

Thanks for the PR @runn-vermel! I'd like to discuss the bigger problem in #26 and see if we can find a solution that doesn't involve adding/documenting/relying on adding more events to solve the race condition.

@runn-vermel
Copy link
Author

haha - I completely understand. This is a band-aid, for sure.

But... it's a band aid that works NOW. I fully agree this should probably be re-written, but in the meantime....

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

Successfully merging this pull request may close these issues.

3 participants