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.

Sign up for GitHub

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

Jump to bottom

[FIX] Notification sounds #17616

Merged
merged 3 commits into from
May 12, 2020
Merged

[FIX] Notification sounds #17616

merged 3 commits into from
May 12, 2020

Conversation

sampaiodiego
Copy link
Member

@sampaiodiego sampaiodiego commented May 12, 2020 •
edited by rodrigok
Loading

  • Global CDN config was ignored when loading the sound files
  • Upload of custom sounds wasn't getting the file extension correctly
  • Some translations were missing
  • Edit and delete of custom sounds were not working correctly
  • fix new message sound
  • fix new room sound
  • use CDN for sounds
  • fix custom sounds uploads missing extension
  • fix custom sounds missing i18n keys
  • send correct custom sounds file type header
  • fix custom sounds delete
  • fix custom sound edit
  • should we separate "new room" from "new message" sounds? the way I changed they're put together.. so people can chose the "door" sound to new messages (reference)

@sampaiodiego sampaiodiego added this to the 3.3.0 milestone May 12, 2020
@sampaiodiego sampaiodiego requested a review from ggazzo May 12, 2020 02:58
@ggazzo ggazzo force-pushed the fix-notification-sounds branch from be53c3a to 0a1fa7b Compare May 12, 2020 04:26
@rodrigok
Copy link
Member

should we separate "new room" from "new message" sounds? the way I changed they're put together.. so people can chose the "door" sound to new messages (reference)

I think it's ok

@ggazzo ggazzo marked this pull request as ready for review May 12, 2020 19:34
@ggazzo ggazzo merged commit a09b606 into develop May 12, 2020
@ggazzo ggazzo deleted the fix-notification-sounds branch May 12, 2020 19:37
ggazzo added a commit that referenced this pull request May 12, 2020
@ggazzo
Merge branch 'remove-get-dom' of github.com:RocketChat/Rocket.Chat in…
b6ebf23 
…to remove-get-dom

* 'remove-get-dom' of github.com:RocketChat/Rocket.Chat:
  [FIX] Notification sounds (#17616)
  [FIX] Resolve 'app already exists' error on app update (#17544)
  [NEW] Unread bars on sidebar (#16862)
  [FIX] Relative image path in oembededUrlWidget (#15902)
  Regression: Do not show custom status inside sequential messages (#17613)
gabriellsh added a commit that referenced this pull request May 13, 2020
@gabriellsh
Merge branch 'integrations' of github.com:RocketChat/Rocket.Chat into…
06da63c 
… integrations

* 'integrations' of github.com:RocketChat/Rocket.Chat:
  [NEW] Add Permissions to deal with Omnichannel visitor past chats history (#17580)
  [NEW] Add permissions to deal with Omnichannel custom fields (#17567)
  [FIX] Livechat iframe allow microphone and camera (#9956)
  [FIX] Do not allow passwords on private channels (#15642)
  [FIX] Mail Messages > Cannot mail own user. (#17625)
  [FIX] remove multiple options from dontAskMeAgain (#17514)
  [FIX] Notification sounds (#17616)
  [FIX] Resolve 'app already exists' error on app update (#17544)
  [NEW] Unread bars on sidebar (#16862)
  [FIX] Relative image path in oembededUrlWidget (#15902)
  Regression: Do not show custom status inside sequential messages (#17613)
This was referenced May 28, 2020
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rodrigok rodrigok rodrigok approved these changes

@ggazzo ggazzo Awaiting requested review from ggazzo

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
3.3.0
Development

Successfully merging this pull request may close these issues.
3 participants
@sampaiodiego @rodrigok @ggazzo