fix: Updating the logo in the app editor to use favicon instead #41147
Conversation
WalkthroughThe Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🧰 Additional context used🧠 Learnings (2)📓 Common learningsapp/client/src/utils/BrandingUtils.ts (7)Learnt from: ankitakinger Learnt from: ankitakinger Learnt from: alex-golovanov Learnt from: alex-golovanov Learnt from: abhvsn Learnt from: abhvsn Learnt from: sharat87 ⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
🔇 Additional comments (1)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
|
/build-deploy-preview skip-tests=true |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/16591090114. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
app/client/src/pages/Editor/AppsmithLink.tsx (1)
52-53: Change from brandLogoUrl to brandFaviconUrl looks good technically.The implementation is correct and maintains the existing fallback logic. However, consider that favicons are typically optimized for small browser display and may not provide the best visual quality when used as logos in the UI.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/client/src/pages/Editor/AppsmithLink.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: ankitakinger
PR: appsmithorg/appsmith#29965
File: app/client/src/ce/sagas/ApplicationSagas.tsx:736-742
Timestamp: 2024-07-26T21:12:57.228Z
Learning: The use of `editorId` instead of `appId` is an intentional change in the PR and is part of reverting to a previous functionality.
Learnt from: sharat87
PR: appsmithorg/appsmith#30252
File: deploy/docker/fs/usr/lib/python3/dist-packages/supervisor/appsmith_supervisor_stdout.py:21-29
Timestamp: 2024-07-26T21:12:57.228Z
Learning: The user has confirmed that the suggested changes to handle potential exceptions and improve the robustness of the `main` function in `appsmith_supervisor_stdout.py` are acceptable.
Learnt from: Aishwarya-U-R
PR: appsmithorg/appsmith#29405
File: app/client/cypress/e2e/Regression/ClientSide/Binding/TableV2_Widget_API_Pagination_spec.js:37-41
Timestamp: 2024-07-26T21:12:57.228Z
Learning: The pull request titled "test: Cypress | Replace static with Dynamic waits - Part 1" is part of a phased approach where only certain test specifications are targeted for static wait removal in the initial phase. Future phases will address additional specs.
Learnt from: Aishwarya-U-R
PR: appsmithorg/appsmith#29405
File: app/client/cypress/e2e/Regression/ClientSide/Binding/TableV2_Widget_API_Pagination_spec.js:37-41
Timestamp: 2024-10-08T15:32:39.374Z
Learning: The pull request titled "test: Cypress | Replace static with Dynamic waits - Part 1" is part of a phased approach where only certain test specifications are targeted for static wait removal in the initial phase. Future phases will address additional specs.
Learnt from: CR
PR: appsmithorg/appsmith#0
File: .cursor/rules/index.mdc:0-0
Timestamp: 2025-07-21T07:25:40.986Z
Learning: Pull request titles must follow the Conventional Commits specification (e.g., type(scope): description)
Learnt from: CR
PR: appsmithorg/appsmith#0
File: .cursor/rules/README.md:0-0
Timestamp: 2025-07-21T07:25:06.064Z
Learning: Pull request titles must follow semantic conventions as described in 'semantic-pr.md'
Learnt from: abhvsn
PR: appsmithorg/appsmith#36458
File: deploy/docker/fs/opt/appsmith/baseline-ce/plugin.jsonl:17-18
Timestamp: 2024-09-20T14:55:06.929Z
Learning: In the product, icon URLs are referred to using 'https://s3.us-east-2.amazonaws.com/assets.appsmith.com/...'.
Learnt from: abhvsn
PR: appsmithorg/appsmith#36458
File: deploy/docker/fs/opt/appsmith/baseline-ce/plugin.jsonl:17-18
Timestamp: 2024-10-08T15:32:34.115Z
Learning: In the product, icon URLs are referred to using 'https://s3.us-east-2.amazonaws.com/assets.appsmith.com/...'.
Learnt from: ankitakinger
PR: appsmithorg/appsmith#37330
File: app/client/src/pages/common/SearchBar/HomepageHeaderAction.tsx:95-95
Timestamp: 2024-11-12T11:42:28.998Z
Learning: In icon provider components within the TypeScript/React codebase, old settings icons like `"settings-2-line"` and `"settings-control"` are intentionally provided alongside new icons. These references are acceptable and should not be flagged for updates.
Learnt from: ankitakinger
PR: appsmithorg/appsmith#37330
File: app/client/src/pages/Editor/gitSync/components/GitChangesList/StaticChange.tsx:52-52
Timestamp: 2024-11-12T07:37:42.598Z
Learning: The icon provider components in `app/client/packages/design-system/widgets-old/src/Icon/index.tsx` and `app/client/packages/design-system/ads/src/Icon/Icon.provider.tsx` should provide both `settings-2-line` and `settings-v3` icons and should not be updated to remove the old icon.
Learnt from: brayn003
PR: appsmithorg/appsmith#40462
File: app/client/src/git/components/ImportOverrideModal/ImportOverrideModalView.tsx:34-40
Timestamp: 2025-04-28T16:16:02.155Z
Learning: The Appsmith team prefers not to include HTML markup in string constants. Text styling or emphasis should be handled through proper React components rather than HTML tags in strings.
app/client/src/pages/Editor/AppsmithLink.tsx (11)
Learnt from: vhemery
PR: #37371
File: app/client/src/widgets/ImageWidget/widget/index.tsx:382-382
Timestamp: 2024-11-13T09:13:11.481Z
Learning: In the ImageWidget component (app/client/src/widgets/ImageWidget/widget/index.tsx), the alt property should remain optional (alt?: string;) because there are fallback values when the user does not provide alternative text.
Learnt from: ankitakinger
PR: #37330
File: app/client/src/pages/Editor/gitSync/components/GitChangesList/StaticChange.tsx:52-52
Timestamp: 2024-11-12T07:37:42.598Z
Learning: The icon provider components in app/client/packages/design-system/widgets-old/src/Icon/index.tsx and app/client/packages/design-system/ads/src/Icon/Icon.provider.tsx should provide both settings-2-line and settings-v3 icons and should not be updated to remove the old icon.
Learnt from: ankitakinger
PR: #37330
File: app/client/src/pages/common/SearchBar/HomepageHeaderAction.tsx:95-95
Timestamp: 2024-11-12T11:42:28.998Z
Learning: In icon provider components within the TypeScript/React codebase, old settings icons like "settings-2-line" and "settings-control" are intentionally provided alongside new icons. These references are acceptable and should not be flagged for updates.
Learnt from: abhvsn
PR: #36458
File: deploy/docker/fs/opt/appsmith/baseline-ce/plugin.jsonl:17-18
Timestamp: 2024-09-20T14:55:06.929Z
Learning: In the product, icon URLs are referred to using 'https://s3.us-east-2.amazonaws.com/assets.appsmith.com/...'.
Learnt from: abhvsn
PR: #36458
File: deploy/docker/fs/opt/appsmith/baseline-ce/plugin.jsonl:17-18
Timestamp: 2024-10-08T15:32:34.115Z
Learning: In the product, icon URLs are referred to using 'https://s3.us-east-2.amazonaws.com/assets.appsmith.com/...'.
Learnt from: KelvinOm
PR: #29387
File: app/client/packages/design-system/widgets/src/components/TagGroup/src/Tag.tsx:9-9
Timestamp: 2024-10-08T15:32:34.114Z
Learning: The CloseIcon is being moved to a common directory for better reusability across components, following the suggestion to avoid importing it from the Modal component's directory.
Learnt from: KelvinOm
PR: #29387
File: app/client/packages/design-system/widgets/src/components/TagGroup/src/Tag.tsx:9-9
Timestamp: 2024-07-26T21:12:57.228Z
Learning: The CloseIcon is being moved to a common directory for better reusability across components, following the suggestion to avoid importing it from the Modal component's directory.
Learnt from: brayn003
PR: #38088
File: app/client/src/git/components/GitQuickActions/BranchButton/index.tsx:72-74
Timestamp: 2024-12-11T08:33:24.352Z
Learning: In the 'BranchButton' component in 'app/client/src/git/components/GitQuickActions/BranchButton/index.tsx' (TypeScript, React), the useEffect hook that checks for label ellipsis does not need to include currentBranch in its dependency array.
Learnt from: brayn003
PR: #38171
File: app/client/src/git/components/DefaultBranch/DefaultBranchCE.tsx:1-14
Timestamp: 2024-12-15T17:45:48.303Z
Learning: In app/client/src/git/components/DefaultBranch/DefaultBranchCE.tsx, the feature flag check is performed at a higher level, so it's acceptable to have isGitProtectedFeatureLicensed={false} in this component.
Learnt from: sharat87
PR: #37715
File: app/client/packages/rts/src/ctl/backup/links/DiskSpaceLink.ts:7-7
Timestamp: 2024-11-28T07:22:10.857Z
Learning: In the DiskSpaceLink class in app/client/packages/rts/src/ctl/backup/links/DiskSpaceLink.ts, it's acceptable to hardcode the backup path to /appsmith-stacks.
Learnt from: brayn003
PR: #40462
File: app/client/src/git/components/ImportOverrideModal/ImportOverrideModalView.tsx:34-40
Timestamp: 2025-04-28T16:16:02.155Z
Learning: The Appsmith team prefers not to include HTML markup in string constants. Text styling or emphasis should be handled through proper React components rather than HTML tags in strings.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: perform-test / client-build / client-build
- GitHub Check: perform-test / rts-build / build
- GitHub Check: client-check-cyclic-deps / check-cyclic-dependencies
- GitHub Check: client-lint / client-lint
- GitHub Check: client-unit-tests / client-unit-tests
- GitHub Check: client-prettier / prettier-check
- GitHub Check: client-build / client-build
|
/build-deploy-preview skip-tests=true |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/16591390761. |
|
Deploy-Preview-URL: https://ce-41147.dp.appsmith.com |
|
/build-deploy-preview skip-tests=true |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/16592018058. |
|
Deploy-Preview-URL: https://ce-41147.dp.appsmith.com |
## Description Updating the logo in the app editor to use favicon instead Fixes [#41134](#41134) ## Automation /ok-to-test tags="@tag.Sanity" ### 🔍 Cypress test results <!-- This is an auto-generated comment: Cypress test results --> > [!IMPORTANT] > 🟣 🟣 🟣 Your tests are running. > Tests running at: <https://github.com/appsmithorg/appsmith/actions/runs/16591754385> > Commit: 29ca678 > Workflow: `PR Automation test suite` > Tags: `@tag.Sanity` > Spec: `` > <hr>Tue, 29 Jul 2025 09:11:02 UTC <!-- end of auto-generated comment: Cypress test results --> ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [ ] No <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Updated the logo in the Appsmith link to display the organization's favicon if available and different from the default, otherwise defaults to the standard logo. * **Bug Fixes** * Increased the maximum allowed favicon size in branding settings from 32x32 to 48x48 pixels, with updated validation and messaging. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
fix: Updating the logo in the app editor to use favicon instead (#41147)
Description
Updating the logo in the app editor to use favicon instead
Fixes #41134
Automation
/ok-to-test tags="@tag.Sanity"
🔍 Cypress test results
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/16591754385
Commit: 29ca678
Cypress dashboard.
Tags:
@tag.SanitySpec:
Tue, 29 Jul 2025 09:56:28 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit