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(TopBar): add next meeting date #12902

Merged
merged 1 commit into from
Aug 15, 2024

Conversation

DorraJaouad
Copy link
Contributor

@DorraJaouad DorraJaouad commented Aug 5, 2024

☑️ Resolves

🖌️ UI Checklist

🖼️ Screenshots / Screencasts

🏚️ Before 🏡 After
image image

🏁 Checklist

  • 🌏 Tested with Chrome, Firefox and Safari or should not be risky to browser differences
  • 🖥️ Tested with Desktop client or should not be risky for it
  • 🖌️ 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 added this to the 💙 Next Beta (30) milestone Aug 5, 2024
@DorraJaouad DorraJaouad self-assigned this Aug 5, 2024
@DorraJaouad DorraJaouad force-pushed the feat/5909/calendar-event-exposure branch 3 times, most recently from bac8c7d to 1fbbb94 Compare August 8, 2024 09:57
@nickvergessen
Copy link
Member

Something missing?

@DorraJaouad
Copy link
Contributor Author

DorraJaouad commented Aug 13, 2024

Something missing?

The url parameter to calendar from API. Will test today (Done)

@ChristophWurst
Copy link
Member

👍 Tested and works 👏

I noticed a bug with full day events. The start time is relative to UTC, so full day events show at 2AM for events in CEST. I'll try to fix thin in the API.

@DorraJaouad DorraJaouad force-pushed the feat/5909/calendar-event-exposure branch from 1fbbb94 to eacbd39 Compare August 14, 2024 08:18
@DorraJaouad DorraJaouad marked this pull request as ready for review August 14, 2024 08:22
Copy link
Member

@jancborchardt jancborchardt left a comment

Choose a reason for hiding this comment

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

Looks good design-wise, just see my 2 inline comments on semantics. :)

src/components/TopBar/TopBar.vue Outdated Show resolved Hide resolved
src/components/TopBar/TopBar.vue Outdated Show resolved Hide resolved
</div>
<div class="event-info">
<p class="event-info__header">
{{ t('spreed', 'Next call') }}
Copy link
Member

Choose a reason for hiding this comment

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

should it say next meeting or call?
Could be an arranged chat session as well? cant really judge from the event?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Technically, all of them are a call but the topic differs ( meeting, session...)

Copy link
Contributor

Choose a reason for hiding this comment

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

I think, this was about the cases people are not really initiating a video/audio call.

But that really depends on the definition of call:

  • an order or request for someone to be present 📆
  • an instance of speaking to someone on the phone 🤙🏽
  • a brief visit 👀

I find Next call appropriate, but we can also consider Next event, Next meeting, or leave the note to translators what this is about, to represent correctly on another languages

Copy link
Member

Choose a reason for hiding this comment

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

"Next call" is nice cause the button next to it says "Start call" so the language is the same.
But e.g. if we have a Talk conversation for the conference and the conference event is linked to that, it’s just "Next event". So that would be more generically applicable, to cover all sorts of cases.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Knowing f a conversation was made for a conference or conference-like event is a bit tricky from info given in hand. Yet, We can use "Next event" if :

  • Conversation is a public conversation
    OR
  • Conversation has more than 50 participants (cannot be a team room)
    OR
  • the meeting is full day (cannot be a meeting or a call)

src/components/TopBar/TopBar.vue Outdated Show resolved Hide resolved
src/stores/chatExtras.js Outdated Show resolved Hide resolved
src/components/TopBar/TopBar.vue Outdated Show resolved Hide resolved
src/stores/chatExtras.js Outdated Show resolved Hide resolved
src/components/TopBar/TopBar.vue Outdated Show resolved Hide resolved
src/services/conversationsService.js Outdated Show resolved Hide resolved
Signed-off-by: DorraJaouad <dorra.jaoued7@gmail.com>
@DorraJaouad DorraJaouad force-pushed the feat/5909/calendar-event-exposure branch from 4688013 to cba69ad Compare August 14, 2024 11:45
@nickvergessen
Copy link
Member

/backport to stable30

@nickvergessen nickvergessen merged commit 6e0d3be into main Aug 15, 2024
46 checks passed
@nickvergessen nickvergessen deleted the feat/5909/calendar-event-exposure branch August 15, 2024 07:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Show upcoming events for the current conversation inside Talk
6 participants