-
Notifications
You must be signed in to change notification settings - Fork 4.2k
fix: Updating the logo in App editor to use branding logo instead #41135
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.
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
Conversation
WalkthroughThe Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~7 minutes Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ 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
|
There was a problem hiding this 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
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/client/src/pages/Editor/AppsmithLink.tsx
(3 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
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: 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: 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: 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.
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.
app/client/src/pages/Editor/AppsmithLink.tsx (12)
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: jacquesikot
PR: #40967
File: app/client/src/components/OrganizationDropdown.tsx:190-206
Timestamp: 2025-06-18T06:15:59.605Z
Learning: The OrganizationDropdown component in app/client/src/components/OrganizationDropdown.tsx is intentionally hidden on mobile devices. The LeftPane component that contains it returns null when isMobile is true, as stated in the PR objectives for better mobile user experience.
Learnt from: brayn003
PR: #38088
File: app/client/src/git/components/GitContextProvider/hooks/useGitBranches.ts:40-43
Timestamp: 2024-12-11T08:25:39.197Z
Learning: In app/client/src/git/components/GitContextProvider/hooks/useGitBranches.ts
, the useMemo
hook includes dependencies artifactType
and baseArtifactId
in its dependency array.
Learnt from: brayn003
PR: #38155
File: app/client/src/git/store/selectors/gitConfigSelectors.ts:7-9
Timestamp: 2024-12-14T22:24:11.946Z
Learning: The selector selectFetchGlobalProfileState
in gitConfigSelectors.ts
is already memoized.
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: 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: 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: 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: 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.
Learnt from: sharat87
PR: #37551
File: app/client/src/api/interceptors/response/failureHandlers/handleBadRequestError.ts:4-4
Timestamp: 2024-11-25T09:15:44.157Z
Learning: In the Appsmith codebase, always import getAppsmithConfigs
from ee/configs
because EE configs extend from CE configs.
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/...'.
⏰ 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). (8)
- GitHub Check: perform-test / rts-build / build
- GitHub Check: perform-test / client-build / client-build
- GitHub Check: perform-test / server-build / server-unit-tests
- GitHub Check: client-lint / client-lint
- GitHub Check: client-unit-tests / client-unit-tests
- GitHub Check: client-check-cyclic-deps / check-cyclic-dependencies
- GitHub Check: client-build / client-build
- GitHub Check: client-prettier / prettier-check
🔇 Additional comments (3)
app/client/src/pages/Editor/AppsmithLink.tsx (3)
8-9
: LGTM! Clean Redux integration.The imports are correctly added and follow the established patterns in the codebase.
25-25
: Good addition for logo aspect ratio handling.The
object-fit: contain
property ensures custom brand logos maintain their aspect ratio and fit properly within the container.
30-31
: Proper Redux integration.The useSelector hook is correctly implemented to retrieve organization configuration from the global state.
Description
Updating the logo in App editor to use branding logo 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/16554878901
Commit: c7818d7
Cypress dashboard.
Tags:
@tag.Sanity
Spec:
Sun, 27 Jul 2025 20:37:57 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit