- 
                Notifications
    
You must be signed in to change notification settings  - Fork 277
 
feat(gathering): message copyier #5013
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
| 
           The latest updates on your projects. Learn more about Vercel for GitHub. 
  | 
    
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.
Fingers crossed it does't affect chrome review
        
          
                packages/extension/src/companion/suggestion/MessageSuggestion.tsx
              
                Outdated
          
            Show resolved
            Hide resolved
        
              
          
                packages/extension/src/companion/suggestion/MessageSuggestionPopup.tsx
              
                Outdated
          
            Show resolved
            Hide resolved
        
      Co-authored-by: Ole-Martin Bratteng <[email protected]>
| const [popups, setPopups] = useState<MessagePopup[]>([]); | ||
| const [observer] = useState( | ||
| new MutationObserver(async () => { | ||
| await new Promise((resolve) => setTimeout(resolve, 10)); // wait for the dom to update | 
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.
extract this into a util we can re-use. could name it sleep.
export const sleep = (time: number) => new Promise((resolve) => setTimeout(resolve, time))| const toUpdate = []; | ||
| 
               | 
          ||
| bubbles.forEach((bubble) => { | 
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.
does bubbles.map work? Then we can just do const toUpdate = bubbles.map.....
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.
Yeah that can work. Will just have to add .filter for ids that are not found. The additional iteration shouldn't impace the performance that much.
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.
LGTM, haven't tested it yet though.
Changes
Events
Did you introduce any new tracking events?
Experiment
Did you introduce any new experiments?
Manual Testing
Caution
Please make sure existing components are not breaking/affected by this PR
Preview domain
https://feat-message-copyier.preview.app.daily.dev