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

feat(call): add option to enable blur background by default for all c… #13490

Merged
merged 9 commits into from
Nov 15, 2024
Merged

feat(call): add option to enable blur background by default for all c… #13490

merged 9 commits into from
Nov 15, 2024

Conversation

DorraJaouad
Copy link
Contributor

@DorraJaouad DorraJaouad commented Oct 7, 2024 •
edited
Loading

…onversations

☑️ Resolves

It's default settings, meaning that if this config is on, the blur effect will apply automatically and it's up to user to change it before joining the call.

🖌️ UI Checklist

🖼️ Screenshots / Screencasts

Talk Settings Media Settings
blur-settings blur-media-settings

🚧 Tasks

  • Clean previous browserStorage usage

🏁 Checklist

  • 🌏 Tested with different browsers / clients:
    • Chromium (Chrome / Edge / Opera / Brave)
    • Firefox
    • Safari
    • Talk Desktop
    • Not risky to browser differences / client
  • 🖌️ Design was reviewed, approved or inspired by the design team
  • ⛑️ Tests are included or not possible
  • 📗 User documentation in https://github.com/nextcloud/documentation/tree/master/user_manual/talk has been updated or is not required

@DorraJaouad DorraJaouad self-assigned this Oct 7, 2024
@DorraJaouad DorraJaouad marked this pull request as draft October 7, 2024 17:39
@nickvergessen
Copy link
Member

I don't recall what we store in the local storage so far. its a "type" (string?) and in case type is image we also store the "name of the image" (string?) or what do we store?

@DorraJaouad
Copy link
Contributor Author

We store 3 info: enabled, type, and (url) if it is an image
image

@nickvergessen
Copy link
Member

We store all 3 in the backend in the future?

@DorraJaouad
Copy link
Contributor Author

We store all 3 in the backend in the future?

That's a better behavior IMO (not reliant on browser). Will it stay per conversation level or we change to one setting for all conversations?

@nickvergessen
Copy link
Member

The API stored config has to be 1 for all, otherwise we save too many preferences?

@DorraJaouad DorraJaouad added 3. to review enhancement feature: api 🛠️ OCS API for conversations, chats and participants feature: frontend 🖌️ "Web UI" client feature: call 📹 Voice and video calls labels Oct 31, 2024
@DorraJaouad DorraJaouad requested a review from Antreesy October 31, 2024 19:52
@DorraJaouad DorraJaouad marked this pull request as ready for review November 2, 2024 09:58
Antreesy
Antreesy reviewed Nov 4, 2024
View reviewed changes
src/stores/settings.js Outdated Show resolved Hide resolved
src/composables/useDevices.js Outdated Show resolved Hide resolved
src/components/SettingsDialog/MediaDevicesPreview.vue Outdated Show resolved Hide resolved
src/components/MediaSettings/MediaSettings.vue Outdated Show resolved Hide resolved
docs/capabilities.md Outdated Show resolved Hide resolved
Antreesy
Antreesy reviewed Nov 11, 2024
View reviewed changes
docs/settings.md Outdated Show resolved Hide resolved
src/components/SettingsDialog/MediaDevicesPreview.vue Outdated Show resolved Hide resolved
src/components/MediaSettings/MediaSettings.vue Outdated Show resolved Hide resolved
src/components/MediaSettings/VideoBackgroundEditor.vue Outdated Show resolved Hide resolved
src/composables/useDevices.js Outdated Show resolved Hide resolved
@DorraJaouad DorraJaouad force-pushed the feat/add-blur-config branch 2 times, most recently from bd360b6 to 43a27db Compare November 11, 2024 15:19
Antreesy
Antreesy approved these changes Nov 13, 2024
View reviewed changes
Copy link
Contributor

@Antreesy Antreesy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Acceptable

lib/Config.php Outdated Show resolved Hide resolved
lib/Controller/RoomController.php Outdated Show resolved Hide resolved
docs/settings.md Outdated Show resolved Hide resolved
src/stores/settings.js Outdated Show resolved Hide resolved
src/components/MediaSettings/MediaSettings.vue Outdated Show resolved Hide resolved
src/components/MediaSettings/VideoBackgroundEditor.vue Outdated Show resolved Hide resolved
@DorraJaouad
feat(call): add option to enable blur background by default for all c…
59349e7 
…onversations

Signed-off-by: DorraJaouad <dorra.jaoued7@gmail.com>
@DorraJaouad
Feat: add blur background config
b6cb2d9 
Signed-off-by: DorraJaouad <dorra.jaoued7@gmail.com>
@DorraJaouad
fix: migrate from browser storage to stored value config
d2022b8 
Signed-off-by: DorraJaouad <dorra.jaoued7@gmail.com>
@DorraJaouad
fix: openapi
a456424 
Signed-off-by: DorraJaouad <dorra.jaoued7@gmail.com>
@DorraJaouad
fix(jest): update tests
4b473f0 
Signed-off-by: DorraJaouad <dorra.jaoued7@gmail.com>
@DorraJaouad DorraJaouad force-pushed the feat/add-blur-config branch 2 times, most recently from 8334931 to e47f4ae Compare November 14, 2024 17:45
Antreesy
Antreesy approved these changes Nov 15, 2024
View reviewed changes
Copy link
Contributor

@Antreesy Antreesy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perfecto

Please rebase, remove composer files and re-split into commits (backend -> openapi -> frontend?)

vendor-bin/php-scoper/composer.json Outdated Show resolved Hide resolved
src/components/MediaSettings/VideoBackgroundEditor.vue Outdated Show resolved Hide resolved
@DorraJaouad
fix: rename blur-background to blur-virtual-background
ab54a93 
Signed-off-by: DorraJaouad <dorra.jaoued7@gmail.com>
@DorraJaouad
fix: reduce the global blur implementation against per conversation l…
0c8a6ad 
…evel virtual background

Signed-off-by: DorraJaouad <dorra.jaoued7@gmail.com>
@DorraJaouad
fix: total renaming from blurBackground to blurVirtualBackground
3d7cbd8 
Signed-off-by: DorraJaouad <dorra.jaoued7@gmail.com>
@DorraJaouad
Fix: reset and respect flag used for skipping the global config
9ea1f70 
Signed-off-by: DorraJaouad <dorra.jaoued7@gmail.com>
@nickvergessen nickvergessen added this to the 🖤 Next Major (31) milestone Nov 15, 2024
@nickvergessen
Copy link
Member

/backport to stable30

nickvergessen
nickvergessen approved these changes Nov 15, 2024
View reviewed changes
Copy link
Member

@nickvergessen nickvergessen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PHP code is fine!

@nickvergessen nickvergessen merged commit 5b1c701 into main Nov 15, 2024
70 checks passed
@nickvergessen nickvergessen deleted the feat/add-blur-config branch November 15, 2024 10:21
@backportbot backportbot bot mentioned this pull request Nov 15, 2024
Merged
@Antreesy Antreesy mentioned this pull request Dec 6, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Antreesy Antreesy Antreesy approved these changes

@nickvergessen nickvergessen nickvergessen approved these changes

Assignees

@DorraJaouad DorraJaouad

Labels
3. to review enhancement feature: api 🛠️ OCS API for conversations, chats and participants feature: call 📹 Voice and video calls feature: frontend 🖌️ "Web UI" client
Projects
None yet
Milestone
🖤 Next Major (31)
Development

Successfully merging this pull request may close these issues.
3 participants
@DorraJaouad @nickvergessen @Antreesy