WARNING: THIS SITE IS A MIRROR OF GITHUB.COM / IT CANNOT LOGIN OR REGISTER ACCOUNTS / THE CONTENTS ARE PROVIDED AS-IS / THIS SITE ASSUMES NO RESPONSIBILITY FOR ANY DISPLAYED CONTENT OR LINKS / IF YOU FOUND SOMETHING MAY NOT GOOD FOR EVERYONE, CONTACT ADMIN AT ilovescratch@foxmail.com
Skip to content

feat: display workspace logos in sidebar navigation #41377

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.

Sign up for GitHub

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

Open
salevine wants to merge 4 commits into
base: release
Choose a base branch
from
+108 −0
Open

feat: display workspace logos in sidebar navigation #41377

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9fea804
feat: display workspace logos in sidebar navigation
salevine Nov 13, 2025
d221389
Fix prettier lint errors
salevine Nov 13, 2025
f86707a
Fixed check typ e errors.
salevine Nov 13, 2025
f4777c5
Address comments - fixed logo size and removed tool tip
salevine Nov 26, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
88 changes: 88 additions & 0 deletions app/client/src/ce/pages/Applications/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ import {
MenuItem as ListItem,
Text,
TextType,
FontWeight,
} from "@appsmith/ads-old";
import { loadingUserWorkspaces } from "pages/Applications/ApplicationLoaders";
import PageWrapper from "pages/common/PageWrapper";
Expand Down Expand Up @@ -395,6 +396,59 @@ export const textIconStyles = (props: { color: string; hover: string }) => {
`;
};

const WorkspaceItemRow = styled.a<{ disabled?: boolean; selected?: boolean }>`
display: flex;
align-items: center;
justify-content: space-between;
text-decoration: none;
padding: 0px var(--ads-spaces-6);
background-color: ${(props) =>
props.selected ? "var(--ads-v2-color-bg-muted)" : "transparent"};
.${Classes.TEXT} {
color: var(--ads-v2-color-fg);
}
.${Classes.ICON} {
svg {
path {
fill: var(--ads-v2-color-fg);
}
}
}
height: 38px;

${(props) =>
!props.disabled
? `
&:hover {
text-decoration: none;
cursor: pointer;
background-color: var(--ads-v2-color-bg-subtle);
border-radius: var(--ads-v2-border-radius);
}`
: `
&:hover {
text-decoration: none;
cursor: default;
}
`}
`;

const WorkspaceIconContainer = styled.span`
display: flex;
align-items: center;

.${Classes.ICON} {
margin-right: var(--ads-spaces-5);
}
`;

const WorkspaceLogoImage = styled.img`
width: 16px;
height: 16px;
object-fit: contain;
margin-right: var(--ads-spaces-5);
`;
Comment on lines +399 to +450
Copy link
Contributor

@coderabbitai coderabbitai bot Nov 26, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Anchor-based WorkspaceItemRow hurts keyboard accessibility; consider button/div instead

WorkspaceItemRow is a styled.a with an onClick handler but no href. This makes it non-focusable by keyboard by default and semantically misleading (it behaves like a button, not a link). That’s a regression in accessibility compared to the existing ListItem-based implementation.

I’d strongly recommend:

  • Switching to styled.button or styled.div with role="button" + tabIndex={0} and an onKeyDown handler for Enter/Space, or
  • Re-using ListItem and customizing its icon slot instead of re-creating the row.

Also, note that:

  • The disabled prop is styled but never passed from WorkspaceMenuItem, so the disabled styling path is currently unused. Either wire it up (e.g., when isFetchingWorkspaces) or remove it until needed.
🤖 Prompt for AI Agents 
In app/client/src/ce/pages/Applications/index.tsx around lines 399 to 450, the
WorkspaceItemRow is implemented as styled.a without an href which breaks
keyboard accessibility and is semantically incorrect for a click-action; change
it to a focusable, actionable element (preferably styled.button, or styled.div
with role="button", tabIndex={0} and an onKeyDown handler that handles
Enter/Space) so it is keyboard-focusable and operable, update event handling
accordingly (use disabled attribute when using button or prevent key activation
when disabled), and either wire the disabled prop through from WorkspaceMenuItem
(e.g., pass disabled when isFetchingWorkspaces) so the disabled styles are
meaningful or remove the unused disabled styling.


export function WorkspaceMenuItem({
isFetchingWorkspaces,
selected,
Expand All @@ -403,6 +457,7 @@ export function WorkspaceMenuItem({
}: any) {
const history = useHistory();
const location = useLocation();
const [imageError, setImageError] = React.useState(false);

const handleWorkspaceClick = () => {
const workspaceId = workspace?.id;
Expand All @@ -414,8 +469,41 @@ export function WorkspaceMenuItem({
}
};

const handleImageError = () => {
setImageError(true);
};

if (!workspace.id) return null;

const hasLogo = workspace?.logoUrl && !imageError;
const displayText = isFetchingWorkspaces
? workspace?.name
Copy link
Contributor

@rahulbarwal rahulbarwal Nov 19, 2025

Choose a reason for hiding this comment

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

If the workspaces are still being fetched, where would you obtain the workspace name? Wouldn't this be undefined in this case?

Copy link
Contributor Author

@salevine salevine Nov 26, 2025

Choose a reason for hiding this comment

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

@rahulbarwal It should not be undefined. Based on everything I can see, the name comes from the getFetchedWorkspaces. Initially, the workspaces array is empty and isFetchingWorkspaces is true. Nothing would show up. While refreshing, it may contain older data - but then when done fetching, it will refresh over it. In the initial state, it all should be safe

: workspace?.name?.length > 22
? workspace.name.slice(0, 22).concat(" ...")
: workspace?.name;

// Use custom component when there's a logo, otherwise use ListItem
if (hasLogo && !isFetchingWorkspaces) {
return (
<WorkspaceItemRow
className={selected ? "selected-workspace" : ""}
onClick={handleWorkspaceClick}
selected={selected}
>
<WorkspaceIconContainer>
<WorkspaceLogoImage
alt={`${workspace.name} logo`}
onError={handleImageError}
src={workspace.logoUrl}
/>
<Text type={TextType.H5} weight={FontWeight.NORMAL}>
{displayText}
</Text>
</WorkspaceIconContainer>
</WorkspaceItemRow>
);
}

return (
<ListItem
className={selected ? "selected-workspace" : ""}
Expand Down
15 changes: 15 additions & 0 deletions app/client/src/ce/reducers/uiReducers/workspaceReducer.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,21 @@ export const handlers = {

draftState.loadingStates.isSavingWorkspaceInfo = false;
draftState.list = [...workspaces];

// Also update searchEntities if they exist to keep search results in sync
if (draftState.searchEntities?.workspaces) {
const searchWorkspaceIndex =
draftState.searchEntities.workspaces.findIndex(
(workspace: Workspace) => workspace.id === action.payload.id,
);

if (searchWorkspaceIndex !== -1) {
draftState.searchEntities.workspaces[searchWorkspaceIndex] = {
...draftState.searchEntities.workspaces[searchWorkspaceIndex],
...action.payload,
};
}
}
},
[ReduxActionErrorTypes.SAVE_WORKSPACE_ERROR]: (
draftState: WorkspaceReduxState,
Expand Down
5 changes: 5 additions & 0 deletions app/server/appsmith-server/src/main/java/com/appsmith/server/domains/Workspace.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,11 @@ public static String toSlug(String text) {

@JsonView(Views.Public.class)
public String getLogoUrl() {
// If there's no logo, return null instead of constructing a URL like "/api/v1/assets/null"
// This prevents the frontend from making pointless requests to load a non-existent image
if (logoAssetId == null || logoAssetId.isEmpty()) {
return null;
}
return Url.ASSET_URL + "/" + logoAssetId;
}

Expand Down