feat(integration): show out-of-office message in 1-1 conversation by Antreesy · Pull Request #10902 · nextcloud/spreed

Antreesy · 2023-11-14T17:55:02Z

☑️ Resolves

Fix Show out-of-office message in 1:1 conversations #10599
Add Pinia store chatExtras (for further migration of quoteReplyStore, as both related to the input component)

🖌️ UI Checklist

🖼️ Screenshots / Screencasts

Main chat:

Screencast.from.29.11.2023.11.35.50.webm

Sidebar chat:

🚧 Tasks

Move to pinia store
- Add test coverage
Handle errors from server if any

🏁 Checklist

🌏 Tested with Chrome, Firefox and Safari or should not be risky to browser differences
🖌️ 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

marcoambrosini · 2023-11-16T02:12:01Z

@nextcloud/designers should we shrink the width to match the quote component?

Antreesy · 2023-11-20T12:12:01Z

should we shrink the width to match the quote component?

Looks good as well. Also won't overlap with 'Scroll to bottom anymore, so it's one problem less:

Some styles should be adjusted, though, because we don't have to align it with messages anymore

src/components/TopBar/TopBar.vue

src/stores/absence.js

DorraJaouad

I didn't mean to approve 🙈

nimishavijay · 2023-11-20T14:21:46Z

Looks really nice when the width is adjusted! :) 2 small questions/feedback:

What do you think about making both the widths equal to the input field? Was there already a discussion about this during the quote reply feature? It would look something like this (and the OOO notecard can also be the same width):
Instead of the avatar, we could use an icon indicating unavailability as Google does (source: this Zapier post). Maybe this event-busy MDI icon? What do you think? :)

Antreesy · 2023-11-20T15:37:33Z

What do you think about making both the widths equal to the input field? Was there already a discussion about this during the quote reply feature?

Was like this since the beginning: #4584, but I can't see the reasons to align the widths.

Instead of the avatar, we could use an icon indicating unavailability

Also agreed, as we don't have to align it with messages list anymore.

marcoambrosini · 2023-11-28T23:13:04Z

@nimishavijay the avatar is there because there is a message from the person just below (See the first screenshot). This whole message should be expanded by default.

To make this clearer when it's collapsed, we could use a layout similar to the NcListItem component. In the first line we could have the warning, and in the second line a preview of the message.

About the with of both component. Could we create a separate issue and see before and after screenshots? I think I tried that back then and it looked worse.

Antreesy · 2023-11-29T10:08:22Z

Not sure if we really need an avatar here (it takes additional time to load render, and consumes 48px of space on small screen)
I'd suggest at least use the same size as on replies

44px	22px

Also checked different widths, and full one doesn't look good above pill-shaped input IMO

Shrink	Full-width

DorraJaouad · 2023-11-29T11:30:03Z

src/components/NewMessage/NewMessage.vue

+				if (!conversation?.status || conversation.status !== 'dnd'
+					|| !this.isOneToOneConversation || this.chatExtrasStore.absence[this.token] !== undefined) {


Suggested change

if (!conversation?.status || conversation.status !== 'dnd'

|| !this.isOneToOneConversation || this.chatExtrasStore.absence[this.token] !== undefined) {

if (!conversation?.status || conversation.status !== 'dnd'

|| !this.isOneToOneConversation ) {

It is already being checked inside the store
Also, conversation.status !== 'dnd' is also questionable because the status update wasn't updated in some tests while the OoO is there. I am not sure how it is synchronized.

I'm checking here specifically for undefined. If user has standard DnD status, but without "Out of office" it will send a request to server each time you open the tab, and receive null. With that line, if stored value has null, request will be sent only once.

Once status message_id will be exposed (should be 'vacationing'), we can switch to checking it, and refactor this code
cc @nickvergessen should i raise an issue to track it? server or spreed repo?

server, but it's unlikely to happen at this point

In the last 3 attempts , the status wasn't updated.

src/stores/chatExtras.js

DorraJaouad · 2023-11-29T11:43:32Z

src/components/NewMessage/NewMessage.vue


 			<!-- Input area -->
 			<div class="new-message-form__input">
+				<NewMessageAbsenceInfo v-if="userAbsence"


I think we should also check the dates. For example, I can set my OoO proactively from tomorrow and I am still working today. AbsenceInfo should be shown starting from tomorrow and not from now.

Then OoO should be set up from tommorow by BG job, I don't think it has to be double-checked on Talk side

I wasn't sure if it should be from the server side to only render the active OoO or any OoO that exists because dates were given in the response.

Should we use them to show when they will be back for example ? like on Webex

In the next iteration maybe

DorraJaouad · 2023-11-29T11:47:46Z

src/components/NewMessage/NewMessageAbsenceInfo.vue

+@import '../../assets/variables';
+
+.absence-reminder {
+	margin: 0 calc(var(--default-clickable-area) / 2) 12px;


I also agree with the full width as a non-optimal form and shorter width is better. However, I recommend setting the width right at the end of the curve of the input pill shape.

Should be margin 16px I guess

but pill border-radius is already calc(var(--default-clickable-area) / 2). So, technically, 22px is correct (but maybe visually 16px would be better)

nickvergessen · 2023-11-29T11:58:31Z

Btw the short status message is not used anywhere. I guess it should be the text on the actual user status (so upstream issue) or would it appear in the notecard?

DorraJaouad · 2023-11-29T12:11:14Z

Btw the short status message is not used anywhere. I guess it should be the text on the actual user status (so upstream issue) or would it appear in the notecard?

It is not shown anymore in the notecard, only long description is used but we can use the short description as a fallback if long description is empty.

If both are empty, the arrow button should be removed.

marcoambrosini

Full width looks good to me, so no strong opinions from my side on that

DorraJaouad

It is ready 🦅

nickvergessen · 2023-11-30T09:48:45Z

Time to fixup then

Signed-off-by: Maksim Sukharev <[email protected]>

Antreesy · 2023-11-30T10:03:32Z

/backport to stable28

Antreesy added 2. developing enhancement feature: frontend 🖌️ "Web UI" client feature: integration 📦 Integration with 3rd party (chat) service labels Nov 14, 2023

Antreesy added this to the 💙 Next Beta (28) milestone Nov 14, 2023

Antreesy self-assigned this Nov 14, 2023

Antreesy force-pushed the feat/10599/show-out-of-office-message branch 2 times, most recently from 612063a to 6e94192 Compare November 15, 2023 12:56

Antreesy requested a review from marcoambrosini November 15, 2023 12:57

nickvergessen modified the milestones: v18.0.0-beta.3, 💙 Next Beta (28) Nov 16, 2023

Antreesy requested a review from DorraJaouad November 20, 2023 12:12

DorraJaouad approved these changes Nov 20, 2023

View reviewed changes

src/components/TopBar/TopBar.vue Outdated Show resolved Hide resolved

src/stores/absence.js Show resolved Hide resolved

DorraJaouad reviewed Nov 20, 2023

View reviewed changes

nickvergessen modified the milestones: 💙 Next Beta (28), 💞 Next Major (29), 💙 Next RC (28) Nov 23, 2023

nickvergessen added the backport-request label Nov 28, 2023

nickvergessen modified the milestones: 💙 Next RC (28), 💞 Next Major (29) Nov 28, 2023

Antreesy force-pushed the feat/10599/show-out-of-office-message branch from 6e94192 to bd9d73d Compare November 28, 2023 19:34

Antreesy force-pushed the feat/10599/show-out-of-office-message branch from bd9d73d to 6da90ad Compare November 29, 2023 10:42

Antreesy requested a review from DorraJaouad November 29, 2023 10:43

Antreesy marked this pull request as ready for review November 29, 2023 10:44

Antreesy force-pushed the feat/10599/show-out-of-office-message branch from 6da90ad to 94d5772 Compare November 29, 2023 10:45

DorraJaouad reviewed Nov 29, 2023

View reviewed changes

marcoambrosini reviewed Nov 30, 2023

View reviewed changes

DorraJaouad approved these changes Nov 30, 2023

View reviewed changes

Antreesy added 3 commits November 30, 2023 11:02

reserve a space for scrollbar in chat and message form

4748c83

Signed-off-by: Maksim Sukharev <[email protected]>

add Pinia store and service method to fetch absence

4df7b25

Signed-off-by: Maksim Sukharev <[email protected]>

add NewMessageAbsenceInfo.vue for showing absence status

54655bf

Signed-off-by: Maksim Sukharev <[email protected]>

Antreesy force-pushed the feat/10599/show-out-of-office-message branch from ab8b77e to 54655bf Compare November 30, 2023 10:02

Antreesy enabled auto-merge November 30, 2023 10:02

Antreesy merged commit 40b002d into main Nov 30, 2023

Antreesy deleted the feat/10599/show-out-of-office-message branch November 30, 2023 10:48

backportbot-nextcloud bot mentioned this pull request Nov 30, 2023

[stable28] feat(integration): show out-of-office message in 1-1 conversation #11049

Merged

backportbot-nextcloud bot removed the backport-request label Nov 30, 2023

		if (!conversation?.status \|\| conversation.status !== 'dnd'
		\|\| !this.isOneToOneConversation \|\| this.chatExtrasStore.absence[this.token] !== undefined) {

Comments

Conversation

Antreesy commented Nov 14, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

☑️ Resolves

🖌️ UI Checklist

🖼️ Screenshots / Screencasts

🚧 Tasks

🏁 Checklist

Uh oh!

marcoambrosini commented Nov 16, 2023

Uh oh!

Antreesy commented Nov 20, 2023

Uh oh!

Uh oh!

Uh oh!

DorraJaouad left a comment

Choose a reason for hiding this comment

Uh oh!

nimishavijay commented Nov 20, 2023

Uh oh!

Antreesy commented Nov 20, 2023

Uh oh!

marcoambrosini commented Nov 28, 2023

Uh oh!

Antreesy commented Nov 29, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nickvergessen commented Nov 29, 2023

Uh oh!

DorraJaouad commented Nov 29, 2023

Uh oh!

marcoambrosini left a comment

Choose a reason for hiding this comment

Uh oh!

DorraJaouad left a comment

Choose a reason for hiding this comment

Uh oh!

nickvergessen commented Nov 30, 2023

Uh oh!

Antreesy commented Nov 30, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Antreesy commented Nov 14, 2023 •

edited

Loading