Skip to content

feat: add skeleton loading state to ServerAvatar component#7384

Open
Carlos2505dev wants to merge 1 commit into
RocketChat:developfrom
Carlos2505dev:feat/server-avatar-skeleton-loading
Open

feat: add skeleton loading state to ServerAvatar component#7384
Carlos2505dev wants to merge 1 commit into
RocketChat:developfrom
Carlos2505dev:feat/server-avatar-skeleton-loading

Conversation

@Carlos2505dev
Copy link
Copy Markdown

@Carlos2505dev Carlos2505dev commented Jun 6, 2026

Proposed changes

This PR introduces a skeleton loading state to the ServerAvatar component in the WorkspaceView. Previously, when the server avatar image was loading or unavailable, the component would render empty space, which resulted in a subpar user experience and left a // TODO: missing skeleton comment in the codebase.

Changes included:

  • Replace the empty render when image is not available with a SkeletonPlaceholder
  • Mark image prop as optional in the IServerAvatar interface
  • Add Storybook stories covering image, skeleton, and theme variants
  • Add snapshot tests via generateSnapshots utility
  • Remove resolved TODO comment

Issue(s)

N/A (Resolves an existing codebase TODO)

How to test or reproduce

  1. Navigate to the WorkspaceView (the server selection/login screen).
  2. Observe the ServerAvatar component before the image fully loads.
  3. You should see a skeleton placeholder instead of an empty space.
  4. Alternatively, check the newly added Storybook stories by running pnpm storybook:start and navigating to WorkspaceView/ServerAvatar.

Types of changes

  • Bugfix (non-breaking change which fixes an issue)
  • Improvement (non-breaking change which improves a current function)
  • New feature (non-breaking change which adds functionality)
  • Documentation update (if none of the other choices apply)

Checklist

  • I have read the CONTRIBUTING doc
  • I have signed the CLA
  • Lint and unit tests pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works (if applicable)
  • I have added necessary documentation (if applicable)
  • Any dependent changes have been merged and published in downstream modules

Further comments

This PR also increases the test coverage for the ServerAvatar component by adding snapshot tests and Storybook stories, strictly following the established patterns in the project (using generateSnapshots and ThemeContext.Provider).

Summary by CodeRabbit

  • New Features

    • ServerAvatar component now displays a skeleton loading placeholder when no image is provided.
    • Image property is now optional.
  • Tests

    • Added snapshot tests for ServerAvatar component across multiple themes and states.

- Replace the empty render when image is not available with a SkeletonPlaceholder

- Mark image prop as optional in the IServerAvatar interface

- Add Storybook stories covering image, skeleton, and theme variants

- Add snapshot tests via generateSnapshots utility

- Remove resolved TODO comment
@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Jun 6, 2026

CLA assistant check
All committers have signed the CLA.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Jun 6, 2026

Review Change Stack

Walkthrough

ServerAvatar component now renders a themed skeleton placeholder when the image prop is missing, replacing the previous behavior of rendering nothing. Supporting changes include new Storybook stories documenting all avatar variants and skeleton states across themes, plus snapshot tests.

Changes

ServerAvatar skeleton loading support

Layer / File(s) Summary
ServerAvatar component update
app/views/WorkspaceView/ServerAvatar.tsx
IServerAvatar.image becomes optional (image?: string). Component now renders either the avatar Image (when image is truthy) or a SkeletonPlaceholder styled with theme colors and fixed dimensions. New import for react-native-skeleton-placeholder.
Storybook stories and theme context wrapper
app/views/WorkspaceView/ServerAvatar.stories.tsx
Introduces ThemedServerAvatar helper that wraps ServerAvatar in ThemeContext.Provider. Exports stories covering avatar rendering with image, without image, empty image string, and skeleton variants across light, dark, and black themes.
Snapshot test harness
app/views/WorkspaceView/ServerAvatar.test.tsx
Integrates Storybook stories with generateSnapshots utility to generate snapshot tests for all avatar variants.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Suggested labels

type: feature

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and accurately describes the main change: adding a skeleton loading state to the ServerAvatar component, which is the primary purpose of this PR.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (2)
app/views/WorkspaceView/ServerAvatar.stories.tsx (1)

13-21: ⚡ Quick win

Use an interface for props and explicit return types for the story components.

To match TS conventions in this repo, extract the inline props shape into an interface and annotate return types for ThemedServerAvatar and each exported story function.

As per coding guidelines: **/*.{ts,tsx}: “Prefer interfaces over type aliases for defining object shapes in TypeScript” and “add explicit type annotations to function parameters and return types”.

Also applies to: 31-51

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@app/views/WorkspaceView/ServerAvatar.stories.tsx` around lines 13 - 21,
Extract the inline props object for ThemedServerAvatar into a named interface
(e.g., interface ThemedServerAvatarProps) and annotate ThemedServerAvatar with
that interface as its parameter type plus an explicit React.FC or explicit
return type like JSX.Element; then add explicit return type annotations
(JSX.Element) to each exported story function (the story names in this file) so
all component/story functions use the new interface and have explicit return
types; update imports/types if needed to use TSupportedThemes in the interface.

Source: Coding guidelines

app/views/WorkspaceView/ServerAvatar.tsx (1)

33-33: ⚡ Quick win

Add an explicit return type to the component render function.

Please annotate the memoized component callback return type explicitly to align with the TypeScript guideline for explicit parameter/return typing.

As per coding guidelines: **/*.{ts,tsx}: “Use TypeScript for type safety; add explicit type annotations to function parameters and return types”.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@app/views/WorkspaceView/ServerAvatar.tsx` at line 33, The memoized component
callback ServerAvatar lacks an explicit return type; update the declaration of
the React.memo callback for ServerAvatar (the function with signature ({ url,
image }: IServerAvatar) => ...) to include an explicit return type such as :
JSX.Element (or : JSX.Element | null if it can return null) so the component
render function is fully typed per guidelines; leave the existing IServerAvatar
param annotation in place.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@app/views/WorkspaceView/ServerAvatar.tsx`:
- Around line 38-44: ServerAvatar currently renders Image whenever image is
truthy so the SkeletonPlaceholder only shows when image is falsy; change
ServerAvatar to track loading state (e.g., isLoading/isError) and render the
SkeletonPlaceholder while isLoading or isError is true, then render the Image
when loaded successfully. Add Image onLoad to clear isLoading and onError to set
isError (and stop loading), ensure the Image source uses `${url}/${image}` only
when image exists, and keep existing styles (styles.image, colors.strokeLight,
SIZE, BORDER_RADIUS) so the skeleton displays during slow or failed network
loads.

---

Nitpick comments:
In `@app/views/WorkspaceView/ServerAvatar.stories.tsx`:
- Around line 13-21: Extract the inline props object for ThemedServerAvatar into
a named interface (e.g., interface ThemedServerAvatarProps) and annotate
ThemedServerAvatar with that interface as its parameter type plus an explicit
React.FC or explicit return type like JSX.Element; then add explicit return type
annotations (JSX.Element) to each exported story function (the story names in
this file) so all component/story functions use the new interface and have
explicit return types; update imports/types if needed to use TSupportedThemes in
the interface.

In `@app/views/WorkspaceView/ServerAvatar.tsx`:
- Line 33: The memoized component callback ServerAvatar lacks an explicit return
type; update the declaration of the React.memo callback for ServerAvatar (the
function with signature ({ url, image }: IServerAvatar) => ...) to include an
explicit return type such as : JSX.Element (or : JSX.Element | null if it can
return null) so the component render function is fully typed per guidelines;
leave the existing IServerAvatar param annotation in place.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 86187c60-e056-499c-a9fe-2e3542a413d0

📥 Commits

Reviewing files that changed from the base of the PR and between 7605531 and 5a1f477.

📒 Files selected for processing (3)
  • app/views/WorkspaceView/ServerAvatar.stories.tsx
  • app/views/WorkspaceView/ServerAvatar.test.tsx
  • app/views/WorkspaceView/ServerAvatar.tsx
📜 Review details
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{js,ts,jsx,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{js,ts,jsx,tsx}: Use descriptive names for functions, variables, and classes that clearly convey their purpose
Write comments that explain the 'why' behind code decisions, not the 'what'
Keep functions small and focused on a single responsibility
Use const by default, let when reassignment is needed, and avoid var
Prefer async/await over .then() chains for handling asynchronous operations
Use explicit error handling with try/catch blocks for async operations
Avoid deeply nested code; refactor complex logic into helper functions

Files:

  • app/views/WorkspaceView/ServerAvatar.test.tsx
  • app/views/WorkspaceView/ServerAvatar.stories.tsx
  • app/views/WorkspaceView/ServerAvatar.tsx
**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{ts,tsx}: Use TypeScript for type safety; add explicit type annotations to function parameters and return types
Prefer interfaces over type aliases for defining object shapes in TypeScript
Use enums for sets of related constants rather than magic strings or numbers

Use TypeScript with strict mode enabled

Files:

  • app/views/WorkspaceView/ServerAvatar.test.tsx
  • app/views/WorkspaceView/ServerAvatar.stories.tsx
  • app/views/WorkspaceView/ServerAvatar.tsx
**/*.{js,jsx,ts,tsx,json}

📄 CodeRabbit inference engine (CLAUDE.md)

Use Prettier formatting with tabs, single quotes, 130 character line width, no trailing commas, and avoid arrow function parentheses

Files:

  • app/views/WorkspaceView/ServerAvatar.test.tsx
  • app/views/WorkspaceView/ServerAvatar.stories.tsx
  • app/views/WorkspaceView/ServerAvatar.tsx
**/*.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

Enforce ESLint rules from @rocket.chat/eslint-config with React, React Native, TypeScript, and Jest plugins

Files:

  • app/views/WorkspaceView/ServerAvatar.test.tsx
  • app/views/WorkspaceView/ServerAvatar.stories.tsx
  • app/views/WorkspaceView/ServerAvatar.tsx
app/views/**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

Place screen components in 'app/views/' directory

Files:

  • app/views/WorkspaceView/ServerAvatar.test.tsx
  • app/views/WorkspaceView/ServerAvatar.stories.tsx
  • app/views/WorkspaceView/ServerAvatar.tsx
🧠 Learnings (3)
📚 Learning: 2026-04-30T17:07:51.020Z
Learnt from: diegolmello
Repo: RocketChat/Rocket.Chat.ReactNative PR: 7274
File: app/lib/services/voip/MediaCallEvents.ts:0-0
Timestamp: 2026-04-30T17:07:51.020Z
Learning: In this Rocket.Chat React Native codebase, the ESLint rule `no-void: error` is enforced. When you see a promise returned from an async call that is not awaited (a “floating promise”), do not silence it with the `void somePromise()` pattern. Instead, handle the promise explicitly by attaching `.catch(...)` (or otherwise awaiting/handling the error) so unhandled-rejection risks are addressed in a way that satisfies the existing ESLint configuration.

Applied to files:

  • app/views/WorkspaceView/ServerAvatar.test.tsx
  • app/views/WorkspaceView/ServerAvatar.stories.tsx
  • app/views/WorkspaceView/ServerAvatar.tsx
📚 Learning: 2026-06-03T18:44:08.388Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.ReactNative PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-06-03T18:44:08.388Z
Learning: Applies to app/theme.tsx : Use theming context from 'app/theme.tsx'

Applied to files:

  • app/views/WorkspaceView/ServerAvatar.stories.tsx
📚 Learning: 2026-03-15T13:55:42.038Z
Learnt from: Rohit3523
Repo: RocketChat/Rocket.Chat.ReactNative PR: 6911
File: app/containers/markdown/Markdown.stories.tsx:104-104
Timestamp: 2026-03-15T13:55:42.038Z
Learning: In Rocket.Chat React Native, the markdown parser requires a space between the underscore wrapping italic text and a mention sigil (_ mention _ instead of _mention_). Ensure stories and tests that include italic-wrapped mentions follow this form to guarantee proper parsing. Specifically, for files like app/containers/markdown/Markdown.stories.tsx, and any test/content strings that exercise italic-mentions, use the pattern _ mention _ (with spaces) to prevent the mention from being treated as plain text. Validate any test strings or story content accordingly.

Applied to files:

  • app/views/WorkspaceView/ServerAvatar.stories.tsx
🔇 Additional comments (1)
app/views/WorkspaceView/ServerAvatar.test.tsx (1)

1-4: LGTM!

Comment on lines +38 to +44
{image ? (
<Image style={[styles.image, { borderColor: colors.strokeLight }]} source={{ uri: `${url}/${image}` }} />
) : (
<SkeletonPlaceholder backgroundColor={colors.surfaceNeutral} borderRadius={BORDER_RADIUS}>
<SkeletonPlaceholder.Item width={SIZE} height={SIZE} />
</SkeletonPlaceholder>
)}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Show the component and relevant lines
FILE="app/views/WorkspaceView/ServerAvatar.tsx"
sed -n '1,200p' "$FILE" | nl -ba | sed -n '1,200p'

# Specifically show the referenced range (38-44) with context
sed -n '25,70p' "$FILE" | nl -ba | sed -n '1,120p'

# Search for any existing image loading/error handling in this file
rg -n "onLoad(Start|End)|onError|isImageLoading|hasImageError|SkeletonPlaceholder" "$FILE" || true

Repository: RocketChat/Rocket.Chat.ReactNative

Length of output: 121


🏁 Script executed:

#!/bin/bash
set -euo pipefail

FILE="app/views/WorkspaceView/ServerAvatar.tsx"

# Show file with line numbers using awk (since nl isn't available)
awk '{print NR "\t" $0}' "$FILE" | sed -n '1,200p'

# Show the referenced range (around lines 38-44) with context
awk '{print NR "\t" $0}' "$FILE" | sed -n '30,90p'

# Search for existing image loading/error handling in this file
rg -n "onLoad(Start|End)|onError|isImageLoading|hasImageError|SkeletonPlaceholder" "$FILE" || true

Repository: RocketChat/Rocket.Chat.ReactNative

Length of output: 2260


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Search for expo-image Image usage with load/error handlers
rg -n "from 'expo-image'|expo-image" app | head -n 50 || true
rg -n "onLoadStart|onLoadEnd|onError" app -S || true

# Search in entire repo for common expo-image event props
rg -n "onError=" -S . || true
rg -n "onLoad(Start|End)=" -S . || true

Repository: RocketChat/Rocket.Chat.ReactNative

Length of output: 50391


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect a few files that likely use expo-image load/error props
for FILE in \
  "app/containers/ServerItem/index.tsx" \
  "app/containers/Avatar/Avatar.tsx" \
  "app/containers/RoomTypeIcon/OmnichannelRoomIcon.tsx" \
  "app/containers/message/Urls.tsx" \
  "app/containers/message/Components/Attachments/Image/Image.tsx" \
  ; do
  if [ -f "$FILE" ]; then
    echo "===== $FILE ====="
    sed -n '1,220p' "$FILE" | awk '{print NR "\t" $0}' | sed -n '1,220p'
    rg -n "onLoad(Start|End)|onError" "$FILE" || true
  fi
done

Repository: RocketChat/Rocket.Chat.ReactNative

Length of output: 17844


🏁 Script executed:

#!/bin/bash
set -euo pipefail

rg -n "import\s*\{\s*Image\s*\}\s*from\s*'expo-image'" app --glob='*.{ts,tsx}' > /tmp/expo_image_imports.txt || true
# Find onLoadStart/onLoadEnd/onLoad/onError usages in non-snapshot code
rg -n "onLoadStart\s*=|onLoadEnd\s*=|onLoad\s*=|onError\s*=" app --glob='*.{ts,tsx}' > /tmp/image_callbacks.txt || true

# Show a small subset of matches that mention expo-image Image usage patterns
# (filter by nearby presence of "from 'expo-image'" within same file)
python3 - <<'PY'
import re, pathlib

imports=set()
for line in open('/tmp/expo_image_imports.txt','r',encoding='utf-8'):
    # format: path:line:content
    path=line.split(':',1)[0]
    imports.add(path)

matches=[]
for line in open('/tmp/image_callbacks.txt','r',encoding='utf-8'):
    path=line.split(':',1)[0]
    if path in imports:
        matches.append(line.strip())

print("Matches in actual code files that import { Image } from 'expo-image':")
for m in matches[:80]:
    print(m)
print("Total:", len(matches))
PY

Repository: RocketChat/Rocket.Chat.ReactNative

Length of output: 257


🌐 Web query:

expo-image Image props onLoadStart onLoadEnd onError supported

💡 Result:

The expo-image component supports the following event props for tracking image loading status: onLoadStart: Called when the image starts to load [1][2]. onLoadEnd: Called when the image load completes, whether it succeeds or fails [1][2]. onError: Called when an image fetching error occurs [1][2]. It receives an event object of type ImageErrorEventData [1][2]. These props are supported across the platforms that expo-image covers, which include Android, iOS, tvOS, and Web [3][4]. Note that in addition to the standard component prop, there is also an onError handler used within the useImage hook, which provides the error and a retry function [3][4].

Citations:


Show skeleton while the avatar image is loading and on error
app/views/WorkspaceView/ServerAvatar.tsx renders <Image /> whenever image is truthy, with the skeleton placeholder only shown when image is falsy—so slow/failed remote loads can leave the component blank instead of displaying the skeleton.

Suggested fix
+const ServerAvatar = React.memo(({ url, image }: IServerAvatar): React.JSX.Element => {
+	const [isImageLoading, setIsImageLoading] = React.useState<boolean>(Boolean(image));
+	const [hasImageError, setHasImageError] = React.useState<boolean>(false);
+
+	React.useEffect(() => {
+		setIsImageLoading(Boolean(image));
+		setHasImageError(false);
+	}, [url, image]);
+
 	return (
 		<View style={styles.container}>
-			{image ? (
-				<Image style={[styles.image, { borderColor: colors.strokeLight }]} source={{ uri: `${url}/${image}` }} />
-			) : (
+			{(!image || isImageLoading || hasImageError) ? (
 				<SkeletonPlaceholder backgroundColor={colors.surfaceNeutral} borderRadius={BORDER_RADIUS}>
 					<SkeletonPlaceholder.Item width={SIZE} height={SIZE} />
 				</SkeletonPlaceholder>
+			) : null}
+			{image ? (
+				<Image
+					style={[styles.image, { borderColor: colors.strokeLight }]}
+					source={{ uri: `${url}/${image}` }}
+					onLoadStart={() => setIsImageLoading(true)}
+					onLoadEnd={() => setIsImageLoading(false)}
+					onError={() => {
+						setIsImageLoading(false);
+						setHasImageError(true);
+					}}
+				/>
 			)}
 		</View>
 	);
 });
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@app/views/WorkspaceView/ServerAvatar.tsx` around lines 38 - 44, ServerAvatar
currently renders Image whenever image is truthy so the SkeletonPlaceholder only
shows when image is falsy; change ServerAvatar to track loading state (e.g.,
isLoading/isError) and render the SkeletonPlaceholder while isLoading or isError
is true, then render the Image when loaded successfully. Add Image onLoad to
clear isLoading and onError to set isError (and stop loading), ensure the Image
source uses `${url}/${image}` only when image exists, and keep existing styles
(styles.image, colors.strokeLight, SIZE, BORDER_RADIUS) so the skeleton displays
during slow or failed network loads.

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.

2 participants