@sanchitmalhotra126 - addressing comment from #60322 (comment).

FYI - refactored so prop no longer needed based on #60483 (comment).

I must've missed commenting on this...another suggestion as opposed to a necessity, but this is the sort of thing I'd tend to put in a ternary:

setProfaneMessageViewToggle(
  profaneMessageViewToggle  === ProfaneMessageViewToggle.VIEW
    ? ProfaneMessageViewToggle.HIDE
    : ProfaneMessageViewToggle.HIDE
);

Main benefit is to not have the function call in two places.

And, further waxing philosophical as opposed to making a serious suggestion here, state machines can work great for this sort of thing.

// defined..somewhere
const nextMessageState = {
  ProfaneMessageViewToggle.HIDE : ProfaneMessageViewToggle.VIEW,
  ProfaneMessageViewToggle.VIEW : ProfaneMessageViewToggle.HIDE,
}

// and then in the onClick
  onClick = { () => setProfaneMessageViewToggle( nextMessageState[profaneMessageViewToggle] ) }

Since we've only got two states I think sticking with the ternary is smart, this strategy is really most useful for when there are more states. I just figured I'd jot down the concept somewhere. 😄

Oh cool (the nextMessageState idea)! Great for future reference.

Will update to ternary.

I'm sorry, I must have zipped past this too fast last time - isWaitingForChatResponse is redundant here.

Don't get me wrong - it's okay as is, but I'm gonna forge ahead with my suggestion here. isWaitingForChatResponse is already baked into the callback for displayWaitingAnimation. The function would return an undefined if it's false already, so the logical wrapper doesn't add to it.

I'm pretty sure the displayWaitingAnimation function would need an else { return null } to make react happy, but otherwise it'd be fine. Then it could just be { displayWaitingAnimation() }

Taking it a step further, I can see a couple of different ways to refine it further.

• instead of a useCallback, it could be useMemo and skip the function call during the render, just embedding { displayWaitingAnimation }
• I might not write this as a hook at all - I think I'd prefer to have it as a regular function (defined outside the component) that takes a boolean arg (shouldDisplay?), and then it'd be called as { displayWaitingAnimation( isWaitingForChatResponse ) }. That way it's a little more generic (but I doubt this'd ever be re-used externally) and it makes it a little more clear that it depends upon the isWaitingForChatReponse variable. Yes, I'm aware that my earlier comment refactored away the conditional that explicitly had that variable. Refactoring has a lot of backtracking 😅
• Last thing, taking that function-with-arg a step further, it could be pulled out into a separate component (either inline or external). And then maybe it'd be called as <WaitingAnimation shouldDisplay={isWaitingForChatResponse} /> (where this component would basically look exactly like the function). Just to put a component in the display instead of a variable. I tend to skew towards components over inline functions/variables where possible.

Again, not that any of this is necessary, just more musing outloud to see if anything looks preferable.

Making an inline component makes sense to me. I really appreciate how you outlined your thought process here. Thanks!

Oooo - now I can remove unnec showWaitingAnimation prop. Thanks again @thomasoniii !

What's this needed for?

The button that hides/shows the profane user message.

Oh yep, was wondering specifically why we needed the negative margin? Is it to cut into the chat message margin?

oh yeah - so that the button was a little closer to its associate chat message.

Ah interesting - I don't think the chat message itself has any extra margin, so I'm wondering if it might be better to nest the button inside the container div and add a bit of padding/margin/gap there instead? I'm just thinking that the -10px makes some assumptions about how chat messages are spaced out which may potentially differ from app to app?

Yeah - I tried that at first, but then the button and message were set in a row.

So then I tried updating flex-direction to column, but then it messed with the user chat message styling (should be aligned to the right but after style update the message takes up entire width).

Here's what it should look like:

I think I'll leave as-is for now and we can address in a follow-up if you have ideas to resolve this. Thanks!

nice!

Why does this class not need the display: flex and height: 100% in its inner div?

Ahh...when I included

  > div {
    display: flex;
    height: 100%;
  }

The hidden attribute was overridden and the content from both tab panels were displayed.

But I overlooked that now when the 'Test student model' tab is active, auto-scrolling no longer works.

We chatted offline and now using the attribute selector hidden to address this. Will updated - thanks so much!