-
Notifications
You must be signed in to change notification settings - Fork 2
Add support for editing messages with Limelight, where possible #446
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
base: main
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for fxms-skylight ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
|
@dmose Is it possible you've got some malformed JSON on your local? I'm only able to trigger that if I do something like remove the screens array from a message, or put a "spotlight" template on something that isn't a spotlight fr. ex. |
@AllegroFox I did some debugging, and it turns out that this data is coming from Looker, despite the function names: https://github.com/mozilla/skylight/blob/main/app/page.tsx#L150-L152 (though, to be fair, this link is to current What happened, I think, is that the functions were written in a world before we got production data from Looker, and when the refactoring introduced that, the function names didn't change. At the very least we're going to want to rename some functions here so stacks are less confusing. |


Uses
window.postMessageto send message JSON to Limelight, allowing us to handle more message data than we can by encoding a URL. (github.io will truncate URLs after a certain length, which almost all of our messages exceed.)So far I've only enabled this for Spotlights and Infobars in the production table, as these are the types of messages that Limelight currently knows about. We can incrementally add support for more templates to Limelight and add those templates to the
SupportedTypesarray, similarly to how we did for message preview.Unfortunately
window.postMessagerequires exact URLs in order to work, so we won't be able to see this working in the deploy preview. I've tested it extensively locally though, and we can always just remove the links from the table if there are issues after landing.