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

feat: Write Talk URL into location, not description #5395

Merged
merged 1 commit into from
Aug 7, 2023

Conversation

ChristophWurst
Copy link
Member

@ChristophWurst ChristophWurst commented Aug 4, 2023

Fall back to description if location is filled, e.g. for a hybrid event.

This contributes part 2 of #5389.

How to test

  1. Set up Calendar and Talk
  2. Create an event
  3. Click the Create Talk room button

On main there will be an URL inserted into the description.
On this branch the URL will be written to the location if it's empty or not set. If it is, the URL is appended to the description.

Fall back to description if location is filled, e.g. for a hybrid event.

Signed-off-by: Christoph Wurst <christoph@winzerhof-wurst.at>
@codecov
Copy link

codecov bot commented Aug 4, 2023

Codecov Report

Patch coverage: 3.57% and project coverage change: -0.02% ⚠️

Comparison is base (847504c) 22.67% compared to head (7f39c7b) 22.66%.
Report is 19 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #5395      +/-   ##
============================================
- Coverage     22.67%   22.66%   -0.02%     
  Complexity      372      372              
============================================
  Files           237      237              
  Lines         11719    11725       +6     
  Branches       2278     2282       +4     
============================================
  Hits           2657     2657              
- Misses         9062     9068       +6     
Flag Coverage Δ
javascript 13.89% <3.57%> (-0.01%) ⬇️
php 65.47% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed Coverage Δ
src/components/Editor/Invitees/InviteesList.vue 0.00% <0.00%> (ø)
src/mixins/EditorMixin.js 3.61% <0.00%> (ø)
src/services/talkService.js 0.00% <0.00%> (ø)
src/store/calendarObjectInstance.js 0.00% <0.00%> (ø)
src/views/Calendar.vue 0.00% <ø> (ø)
src/views/EditSidebar.vue 0.00% <0.00%> (ø)
src/store/settings.js 88.59% <100.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@kesselb
Copy link
Contributor

kesselb commented Aug 4, 2023

Very nice 👍

I would like to suggest adding a bit of explanation before the link in the description.

Example Teams:

Screenshot from 2023-08-04 18-47-06

(Screenshot is from the imip email but the same text is used for the description ics field)

Example Google:

image

(Screenshot is also from the imip email but the same text is used for the description ics field)

As backup, the link could always be in the description.
If the location field is empty, write the link inside. But always append to the description.

Copy link
Contributor

@kesselb kesselb left a comment

Choose a reason for hiding this comment

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

Works 👍

Left two remarks about the general concept.

@ChristophWurst
Copy link
Member Author

Generating text is fun and games until you have more than one language involved 😃

@kesselb
Copy link
Contributor

kesselb commented Aug 4, 2023

Klicken Sie auf den nachfolgenden Link, um an der Online-Video-Besprechung mit nächster Wolkenverbreitung teilzunehmen ;)

@ChristophWurst ChristophWurst merged commit f3a51c4 into main Aug 7, 2023
38 of 40 checks passed
@ChristophWurst ChristophWurst deleted the feat/talk-room-url-location-not-description branch August 7, 2023 06:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews enhancement New feature request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants