Show confetti in a chat room on command or emoji #5140
Conversation
…ut for users with german translation
t3chguy changed the title | Hello @t3chguy, should i ignore that the the pull request didn't pass the end-to-end tests trying to clone riot-web from our org?
|
| That error can be ignored, it is trying to find matching branches for this contribution falling back to the defaults. The e2e test failure is due to your branch being out of date with I gave you a preliminary review, feel free to reach out with any questions |
| Hello @t3chguy, in the current implementation of running slash commands, i think, they all happen on the sender's side to process the message before sending it, right? in SendMessageComposer the commands are ignored from beeing sent. well in confetti command case, what could be unique in the event itself that the message comes with a command or even is only the command? |
| Some commands send messages, some don't. Maybe you can use |
fix if condition trying (pass the dispatcher to sendHtmlMessage)
…into feature_confetti#14676 � Conflicts: � src/SlashCommands.tsx � src/i18n/strings/de_DE.json
changed the implementation of on.Event.decrypted function
throwing the confetti on the sender's side change sendHtmlMessage to sendTextMessage in slashCommands
| Changes
|
* Made color generation dependant on gradient usage. * Changed a loop to use for of
| Hi @t3chguy @turt2live Can you help us with the last failing check? Is it related to our code and if so where can I find the test that is failing here? thanks! |
| Bob's console has some error |
| @t3chguy thanks for the feedback, we fixed the issue! |
| @@ -0,0 +1,76 @@ | |||
| import React, { FunctionComponent, useEffect, useRef } from 'react'; | |||
There was a problem hiding this comment.
All of these new files need their header comments (usually copy/pasted from another file with the copyright line updated)
| declare global { | ||
| interface Window { | ||
| mozRequestAnimationFrame: any; | ||
| oRequestAnimationFrame: any; | ||
| msRequestAnimationFrame: any; | ||
| } | ||
| } |
There was a problem hiding this comment.
We shouldn't need these types - window.requestAnimationFrame should work.
| window.requestAnimationFrame = (function() { | ||
| return window.requestAnimationFrame || | ||
| window.webkitRequestAnimationFrame || | ||
| window.mozRequestAnimationFrame || | ||
| window.oRequestAnimationFrame || | ||
| window.msRequestAnimationFrame || | ||
| function(callback) { | ||
| return window.setTimeout(callback, this.options.frameInterval); | ||
| }; | ||
| })(); |
| } as Effect<ConfettiOptions>, | ||
| ]; | ||
| | ||
| export default effects; |
There was a problem hiding this comment.
We try to avoid export default for consts, and instead prefer to export them as a type instead. For exported constants like this, we'd generally use export const CHAT_EFFECTS: Effect<{ [key: string]: any }> = [...]; and import it as import { CHAT_EFFECTS } from "...";
| Sorry about the noise here from the Vercel bot, it's safe to ignore. I was trialling a new preview method for PRs, but it went a bit haywire. I have now disabled it, so it shouldn't be an issue any more. |
| Hi @turt2live, thanks for your feedback. All points mentioned are taken care of and ready to be reviewed by you. Wish you a nice weekend! |
path to confetti fixed after refactoring
7 participants
@t3chguy as discussed, the draft pull request for confetti feature.
after the first refresh the confetti is thrown again, because the message is considered still not read!
looking forward to ideas and improvements.
Thanks
Fixes element-hq/element-web#14676