3156: Align Typing Indicator and hide on new message #3159

hannaseithe · 2025-03-19T08:43:59Z

Short Description

I aligned the Typing Indicator with the other messages' position & when a new message from Zammad comes in, the Indicator will be hidden.

Proposed Changes

the Typing Indicator is now aligned with the other messages' position
the Typing Indicator gets hidden when a new message from Zammad comes in
when the Typing Indicator appears we scroll into view

Side Effects

no side Effects that I am aware of

Testing

check that the Indicator is in the right position
check that the Indicator vanishes when a new message from Zammad comes in (not the First one)
check that the chat window scrolls all the way down when the Indicator appears

Resolved Issues

Fixes: #3156

bahaaTuffaha

It's working great.. but it needs a bit of cleaning up 🚀
Also I think it needs a release note 🙂

bahaaTuffaha · 2025-03-20T15:20:44Z

web/src/components/ChatConversation.tsx

@@ -17,6 +17,7 @@ const InitialMessage = styled.div`

 const TypingIndicatorWrapper = styled(Message)`
  width: max-content;
+  margin-left: 33px;


🔧 The robot icon is 24px + 8px gap = 32px
And keep in mind that we started to use a multiply of 4 for ui.

The robot icon width is 25px in my branch - is this a recent change?

"And keep in mind that we started to use a multiply of 4 for ui." What exactly does this mean?

We have a new design coming for integreat you can check it out:
https://www.figma.com/design/o5p3ArtrrmOqG6lJggAQE5/%F0%9F%9F%A8-Designsystem-Integreat-(MUI)?node-id=2831-17369&p=f&t=2YiZzVRBygegXzei-0

Its better to use numbers for styling that is dividable by 4 like 32 or 16 for ex. to keep styling consistent.

So should I change the icon width to 24px then?

I’ve kept it at 33px for now. I think aligning to 'multiples of 4' makes sense once the new design is introduced and the CSS is updated consistently. Changing this now would introduce an inconsistency with the current icon size (25px) rather than improving it.

Since you didn't change this, it is optional. However, I agree with @bahaaTuffaha that we generally prefer to have multiples of 4. Obviously only makes sense if you change the icon size to 24 as well.

On another note however, I think setting a hardcoded width based on some value in some other file is usually not best practice and may break in the future. A better solution would perhaps be to use the whole ChatMessage component normally instead. Seems like that is not easily possible though since it requires a normal message model. So not sure what exactly to do here :/

At least at the moment this is broken on rtl layouts (e.g. arbabic) due to the use of margin-left:

web/src/components/ChatConversation.tsx

bahaaTuffaha · 2025-03-20T15:33:30Z

web/src/components/ChatConversation.tsx

 const ChatConversation = ({ messages, hasError, className }: ChatConversationProps): ReactElement => {
  const { t } = useTranslation('chat')
  const [messagesCount, setMessagesCount] = useState(0)
-  const [typingIndicatorVisible, setTypingIndicatorVisible] = useState(false)


🔧 Undo this line.

bahaaTuffaha · 2025-03-20T15:40:15Z

web/src/components/ChatConversation.tsx

-  useEffect(() => {
-    if (waitingForAnswer) {
-      setTypingIndicatorVisible(true)
-
-      const typingIndicatorTimeout = setTimeout(() => {
-        setTypingIndicatorVisible(false)
-      }, TYPING_INDICATOR_TIMEOUT)
-
-      return () => clearTimeout(typingIndicatorTimeout)
-    }
-    return () => undefined
-  }, [waitingForAnswer])
-


You can reuse the same scrolling functionality for both and adding an optional arg as callback that wont be called unless it finish changing the state.

const scrollToBottom = async (beforeScroll?: () => void) => { if (beforeScroll) { await beforeScroll() } messagesEndRef.current?.scrollIntoView({ behavior: 'smooth' }) } useEffect(() => { if (messagesCount < messages.length) { scrollToBottom() setMessagesCount(messages.length) } }, [messages, messagesCount, typingIndicatorVisible]) useEffect(() => { if (waitingForAnswer) { scrollToBottom(() => setTypingIndicatorVisible(true)) const timeout = setTimeout(() => setTypingIndicatorVisible(false), 60000) return () => clearTimeout(timeout) } setTypingIndicatorVisible(false) }, [waitingForAnswer])

This '() => setTypingIndicatorVisible(true)' is a synchronous function and to my understanding finishes when the state is changed (doesn't wait for rendering). I think the easiest solution is to have the useEffect for 'TypingIndicatorVisible' (because then it is already rendered)

🤔 hmm the function contents are synchronous but am waiting for that callback await beforeScroll() to be done before changing the scrolling ref...

But if you await a synchronous function it just immediately executes it.
From what I understood:

With your version we catch it at the point of state-changed. (Edit: This is actually not true, its only after the state changes are queued)

With my version we are a little bit further down the line: state-changed > virtual DOM changed > actual DOM changed (although not yet rendered)

I have decided to keep the useEffect() logic, in order to trigger the scrolling after the DOM was actually changed. But I have kept the scrolldown function as you suggested it.

hannaseithe · 2025-03-24T11:21:03Z

Also I think it needs a release note 🙂

Where do I create a release note? I havent done that yet (btw this is not a new feature, but a bug fix)

bahaaTuffaha · 2025-03-24T11:44:17Z

Also I think it needs a release note 🙂

Where do I create a release note? I havent done that yet (btw this is not a new feature, but a bug fix)

There is a folder called release-notes copy the file ReleaseNoteTemplate.yml into unreleased folder and adjust what necessary... You can look at examples from before.

If the release note is unnecessary for this bug we can remove it 😄

steffenkleinle · 2025-03-26T12:39:17Z

web/src/components/ChatConversation.tsx

@@ -17,6 +17,7 @@ const InitialMessage = styled.div`

 const TypingIndicatorWrapper = styled(Message)`
  width: max-content;
+  margin-left: 33px;


Since you didn't change this, it is optional. However, I agree with @bahaaTuffaha that we generally prefer to have multiples of 4. Obviously only makes sense if you change the icon size to 24 as well.

On another note however, I think setting a hardcoded width based on some value in some other file is usually not best practice and may break in the future. A better solution would perhaps be to use the whole ChatMessage component normally instead. Seems like that is not easily possible though since it requires a normal message model. So not sure what exactly to do here :/

At least at the moment this is broken on rtl layouts (e.g. arbabic) due to the use of margin-left:

steffenkleinle · 2025-03-26T13:20:35Z

web/src/components/ChatConversation.tsx

+  const scrollToBottom = async (beforeScroll?: () => void | Promise<void>) => {
+    if (beforeScroll) {
+      await beforeScroll()
+    }


🔧 This is never actually used, is it?

Suggested change

const scrollToBottom = async (beforeScroll?: () => void | Promise<void>) => {

if (beforeScroll) {

await beforeScroll()

}

const scrollToBottom = () => {

steffenkleinle · 2025-03-26T13:22:55Z

web/src/components/ChatConversation.tsx

+  useEffect(() => {
+    if (typingIndicatorVisible) {
+      scrollToBottom()
+    }
+  }, [typingIndicatorVisible])


🤔 Why is this necessary?

hannaseithe added 2 commits March 19, 2025 09:32

3156: Align Indicator and hide on new message

e8a9af1

3156: Fix CodeComplexity

bc6610c

hannaseithe marked this pull request as ready for review March 19, 2025 09:10

hannaseithe requested review from steffenkleinle, f1sh1918, LeandraH, bahaaTuffaha and lunars97 as code owners March 19, 2025 09:10

bahaaTuffaha requested changes Mar 20, 2025

View reviewed changes

3156: Add Suggestions

7dbb9e1

hannaseithe force-pushed the 3156-fix-chat-typing-indicator branch from 3baca01 to 7dbb9e1 Compare March 25, 2025 10:11

steffenkleinle reviewed Mar 26, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

3156: Align Typing Indicator and hide on new message #3159

3156: Align Typing Indicator and hide on new message #3159

hannaseithe commented Mar 19, 2025

bahaaTuffaha left a comment

bahaaTuffaha Mar 20, 2025 •

edited

Loading

hannaseithe Mar 24, 2025

bahaaTuffaha Mar 24, 2025 •

edited

Loading

hannaseithe Mar 24, 2025

hannaseithe Mar 25, 2025

steffenkleinle Mar 26, 2025

bahaaTuffaha Mar 20, 2025 •

edited

Loading

bahaaTuffaha Mar 20, 2025

hannaseithe Mar 24, 2025

bahaaTuffaha Mar 24, 2025

hannaseithe Mar 24, 2025 •

edited

Loading

hannaseithe Mar 25, 2025

hannaseithe commented Mar 24, 2025

bahaaTuffaha commented Mar 24, 2025 •

edited

Loading

steffenkleinle Mar 26, 2025

steffenkleinle Mar 26, 2025

steffenkleinle Mar 26, 2025

3156: Align Typing Indicator and hide on new message #3159

Are you sure you want to change the base?

3156: Align Typing Indicator and hide on new message #3159

Conversation

hannaseithe commented Mar 19, 2025

Short Description

Proposed Changes

Side Effects

Testing

Resolved Issues

bahaaTuffaha left a comment

Choose a reason for hiding this comment

bahaaTuffaha Mar 20, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bahaaTuffaha Mar 24, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bahaaTuffaha Mar 20, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hannaseithe Mar 24, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hannaseithe commented Mar 24, 2025

bahaaTuffaha commented Mar 24, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bahaaTuffaha Mar 20, 2025 •

edited

Loading

bahaaTuffaha Mar 24, 2025 •

edited

Loading

bahaaTuffaha Mar 20, 2025 •

edited

Loading

hannaseithe Mar 24, 2025 •

edited

Loading

bahaaTuffaha commented Mar 24, 2025 •

edited

Loading