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 voicemail feature #232

Closed
wants to merge 29 commits into from
Closed

Add voicemail feature #232

wants to merge 29 commits into from

Conversation

C0ffeeCode
Copy link
Contributor

@C0ffeeCode C0ffeeCode commented Jan 6, 2022

Description

Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.

This adds a feature to record audio/voice from inside cinny's room input and send it as a file.
Also, it replaces the upload file button's function to instead open a new menu to select what the user would like to attach, like a file or voice.

Additionally, this fixes a bug due to changed Webpack behavior where the compilation fails once the browser tries to load the page.

Deeper description

  • Fixes bug where webpack does not compile and throws issues once browser loads
    • Added dependency on "url"
    • Add "events" and "url" to webpack.common.js -> resolve -> fallback
  • RoomViewInput: attachment/setAttachment renamed to attachmentOrUi/setAttachmentOrUi
    • Previously: Contains file object
    • Now: Contains file object or a string describing which Attachment UI should be shown (more follows)
      • Checks if attachmentOrUi is an object before sending it
    • Unchanged: The user can remove the attached file (and now also other AttachmentFrame UIs) with the same button
  • Adds AttachmentTypeSelector component
    • A menu where the user can select if they want to attach a file or open the voice recorder UI (more follows)
    • Instructs RoomViewInput to change attachmentOrUi to the chosen UI
  • RoomViewInput: Move attachFile into AttachmentFrame
  • Added AttachmentFrame component
    • Replaces attachFile which now is inside AttachmentFrame
    • Shows desired UI like attached file or UIs like the (for now only) voice recorder
  • Added Timer class to utils
    • Can be paused etc.
    • Needed for VoiceMailRecorder
  • Added VoiceMailRecorder component: Records voice; Can be:
    • Paused and resumed,
    • Restarted (so the user can have one more try without having to reopen the UI)
    • Allows the user to attach his recorded audio as a file to the message
    • Canceled the UI using the same button which allows the user to remove an attached file

Type of change

  • New feature (non-breaking change which adds functionality)
  • Bug fix (non-breaking change which fixes an issue)

Screenshots

Note: The selection of icons can/should be improved

Attachment type selector UI

Voice recorder UI

Voicemail attached as a file (like before)

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation (none)
  • My changes generate no new warnings

Preview: https://61f81bf6fc0a4a0e439400a8--pr-cinny.netlify.app
⚠️ Do you trust the author of this PR? Maybe this build will steal your keys or give you malware. Exercise caution. Use test accounts.

@C0ffeeCode
Copy link
Contributor Author

Examples of the bugs this PR solves:

ERROR in ./src/util/AsyncSearch.js 23:0-34
Module not found: Error: Can't resolve 'events' in 'C:\Users\laure\cinny-other\src\util'

BREAKING CHANGE: webpack < 5 used to include polyfills for node.js core modules by default.
This is no longer the case. Verify if you need this module and configure a polyfill for it.

If you want to include a polyfill, you need to:
        - add a fallback 'resolve.fallback: { "events": require.resolve("events/") }'
        - install 'events'
If you don't want to include a polyfill, you can use an empty module like this:
        resolve.fallback: { "events": false }
 @ ./src/app/organisms/room/PeopleDrawer.jsx 20:0-52 71:22-33
 @ ./src/app/organisms/room/Room.jsx 23:0-42 77:52-64
 @ ./src/app/templates/client/Client.jsx 18:0-45 84:38-42
 @ ./src/app/pages/App.jsx 4:0-48 7:62-68
 @ ./src/index.jsx 5:0-34 7:50-53
Module not found: Error: Can't resolve 'url' in 'C:\Users\laure\cinny-other\node_modules\.pnpm\[email protected][email protected]\node_modules\matrix-js-sdk\lib'

BREAKING CHANGE: webpack < 5 used to include polyfills for node.js core modules by default.
This is no longer the case. Verify if you need this module and configure a polyfill for it.

If you want to include a polyfill, you need to:
        - add a fallback 'resolve.fallback: { "url": require.resolve("url/") }'
        - install 'url'
If you don't want to include a polyfill, you can use an empty module like this:
        resolve.fallback: { "url": false }

- improved code quality
@C0ffeeCode C0ffeeCode marked this pull request as ready for review January 6, 2022 18:55
@deltanedas
Copy link

you might want to squash this bruh

@ajbura
Copy link
Member

ajbura commented Jan 16, 2022

Hi, Thanks for the PR. I have few concerns before reviewing this.

  • It looks like you have updated dependencies to their latest minor versions which means you have different setup locally than upstream (likely because of which you got those errors with modules). Please use npm ci to install same dependencies (that’s why we have package-lock.json in repo).
  • Please don’t make single PR for features as well as bugs.
  • Please use good commit messages, I just can’t review all the commits at once and have no idea what you did in commits.

@C0ffeeCode
Copy link
Contributor Author

Well, I cannot change the commit messages.
However, I undid the changes to dependencies.
I think this PR is ready.

@kfiven kfiven added the needs-redesign Need UI/UX re-design label Feb 4, 2022
@kfiven
Copy link
Collaborator

kfiven commented Feb 4, 2022

The current design implementation does seems to slow the file uploading as well as recording. Let me spin up some alternative design implementation.

@menturion menturion mentioned this pull request Jun 24, 2022
@ajbura ajbura closed this Jan 10, 2025
@github-actions github-actions bot locked and limited conversation to collaborators Jan 10, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
needs-redesign Need UI/UX re-design
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants