-
Notifications
You must be signed in to change notification settings - Fork 7
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
Memo main component, improve initial loader bg and bump version #283
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
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.
👍 Looks good to me! Reviewed everything up to 84ff7e6 in 1 minute and 14 seconds
More details
- Looked at
183
lines of code in2
files - Skipped
0
files when reviewing. - Skipped posting
6
drafted comments based on config settings.
1. kits/connect/src/embed-react.tsx:30
- Draft comment:
For consistency, use DEFAULT_WIDTH constant instead of literal '350' in the width check. - Reason this comment was not posted:
Comment was on unchanged code.
2. kits/connect/src/embed-react.tsx:13
- Draft comment:
Wrapping forwardRef with React.memo is acceptable; ensure this pattern aligns with intended ref behavior. - Reason this comment was not posted:
Confidence changes required:33%
<= threshold50%
None
3. kits/connect/package.json:3
- Draft comment:
Bump version to 0.2.22; ensure the changelog/release notes are updated accordingly. - Reason this comment was not posted:
Confidence changes required:0%
<= threshold50%
None
4. kits/connect/src/embed-react.tsx:13
- Draft comment:
Wrapping the component with React.memo is a good performance optimization. However, ensure that callback props (like onReady) and other non-primitive props are memoized upstream to fully benefit. - Reason this comment was not posted:
Confidence changes required:50%
<= threshold50%
None
5. kits/connect/src/embed-react.tsx:88
- Draft comment:
The inline style now conditionally sets the spinner container background based on params?.theme. Confirm that the 'theme' property is reliably provided and consider using design tokens or constants if more themes will be added. - Reason this comment was not posted:
Confidence changes required:30%
<= threshold50%
None
6. kits/connect/src/embed-react.tsx:44
- Draft comment:
The height fallback only applies when the height prop is a number. If a non-numeric (or absent) value is provided, the computed height may be undefined leading to potential 'undefinedpx' in CSS. Confirm if this behavior is intentional. - Reason this comment was not posted:
Confidence changes required:30%
<= threshold50%
None
Workflow ID: wflow_iRlNmWlUJuQUOAON
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
✨ No issues found! Your code is sparkling clean! ✨ |
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.
👍 Looks good to me! Incremental review on aa84b3f in 35 seconds
More details
- Looked at
17
lines of code in1
files - Skipped
0
files when reviewing. - Skipped posting
5
drafted comments based on config settings.
1. packages/engine-frontend/components/ConnectionsTabContent.tsx:105
- Draft comment:
Consider if 'pt-4' is consistent with other similar components; ensure spacing is intentional. - Reason this comment was not posted:
Confidence changes required:20%
<= threshold50%
None
2. packages/engine-frontend/components/ConnectionsTabContent.tsx:108
- Draft comment:
Check that 'pt-2' provides proper spacing between paragraphs, matching design standards. - Reason this comment was not posted:
Confidence changes required:20%
<= threshold50%
None
3. packages/engine-frontend/components/ConnectionsTabContent.tsx:105
- Draft comment:
Good use of spacing. Ensure that the added 'pt-4' class aligns with overall empty state design. - Reason this comment was not posted:
Confidence changes required:0%
<= threshold50%
None
4. packages/engine-frontend/components/ConnectionsTabContent.tsx:108
- Draft comment:
Padding added for subtitle text ('pt-2'). Confirm it meets design consistency. - Reason this comment was not posted:
Confidence changes required:0%
<= threshold50%
None
5. packages/engine-frontend/components/ConnectionsTabContent.tsx:44
- Draft comment:
Typographical/grammatical improvement: The comment 'Is processing state is used for card loader on reconnect.' is a bit confusing. Consider rephrasing it to something like 'The processing state is used for the card loader on reconnect.' for better clarity. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
Workflow ID: wflow_kOOB0g98nNqI9ojN
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
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.
👍 Looks good to me! Incremental review on f115b1d in 4 minutes and 50 seconds
More details
- Looked at
154
lines of code in8
files - Skipped
0
files when reviewing. - Skipped posting
16
drafted comments based on config settings.
1. connectors/connector-discord/def.ts:20
- Draft comment:
Added vertical 'messaging'; ensure it aligns with the vertical definitions in kits/cdk/verticals.ts. - Reason this comment was not posted:
Confidence changes required:0%
<= threshold50%
None
2. connectors/connector-github/def.ts:27
- Draft comment:
Changed vertical from 'other' to 'developer-tools'. Confirm that 'developer-tools' is correctly defined in verticals.ts. - Reason this comment was not posted:
Confidence changes required:0%
<= threshold50%
None
3. connectors/connector-gong/def.ts:27
- Draft comment:
Updated vertical from 'other' to 'sales-enablement'; ensure consistency with verticals mapping. - Reason this comment was not posted:
Confidence changes required:0%
<= threshold50%
None
4. connectors/connector-intercom/def.ts:27
- Draft comment:
Changed vertical from 'other' to 'communication'. Verify that 'communication' is properly defined in verticals.ts. - Reason this comment was not posted:
Confidence changes required:0%
<= threshold50%
None
5. connectors/connector-jira/def.ts:27
- Draft comment:
Updated vertical from 'other' to 'ticketing'; ensure support in the verticals configuration. - Reason this comment was not posted:
Confidence changes required:0%
<= threshold50%
None
6. connectors/connector-kustomer/def.ts:27
- Draft comment:
Changed vertical from 'other' to 'communication'. Please verify that this mapping is intended. - Reason this comment was not posted:
Confidence changes required:0%
<= threshold50%
None
7. connectors/connector-linear/def.ts:27
- Draft comment:
Updated vertical from 'other' to 'ticketing'; check that 'ticketing' is correctly represented in verticals.ts. - Reason this comment was not posted:
Confidence changes required:0%
<= threshold50%
None
8. kits/cdk/verticals.ts:15
- Draft comment:
Reordered and modified vertical definitions. Verify that removed and reinserted keys (e.g., 'developer-tools', 'sales-enablement') correctly map to intended APIs. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%
<= threshold50%
The comment is asking the PR author to verify that the changes made are correct, which violates the rule against asking for confirmation or verification of intentions. It does not provide a specific suggestion or point out a clear issue with the code.
9. connectors/connector-discord/def.ts:20
- Draft comment:
Added verticals array ('messaging'). Verify that 'messaging' exists in verticals.ts. - Reason this comment was not posted:
Confidence changes required:0%
<= threshold50%
None
10. connectors/connector-github/def.ts:27
- Draft comment:
Changed vertical from ['other'] to ['developer-tools']. Confirm alignment with verticals.ts. - Reason this comment was not posted:
Confidence changes required:0%
<= threshold50%
None
11. connectors/connector-gong/def.ts:27
- Draft comment:
Updated vertical to ['sales-enablement'] for Gong. Ensure this categorization is intentional. - Reason this comment was not posted:
Confidence changes required:0%
<= threshold50%
None
12. connectors/connector-intercom/def.ts:27
- Draft comment:
Replaced vertical ['other'] with ['communication'] for Intercom. Check vertical consistency. - Reason this comment was not posted:
Confidence changes required:0%
<= threshold50%
None
13. connectors/connector-jira/def.ts:27
- Draft comment:
Updated vertical from ['other'] to ['ticketing'] for Jira, matching its domain. - Reason this comment was not posted:
Confidence changes required:0%
<= threshold50%
None
14. connectors/connector-kustomer/def.ts:27
- Draft comment:
Changed vertical to ['communication'] for Kustomer. Note the pending type update per TODO. - Reason this comment was not posted:
Confidence changes required:0%
<= threshold50%
None
15. connectors/connector-linear/def.ts:27
- Draft comment:
Modified vertical to ['ticketing'] for Linear. Confirm this aligns with its functionality. - Reason this comment was not posted:
Confidence changes required:0%
<= threshold50%
None
16. kits/cdk/verticals.ts:14
- Draft comment:
Restructured vertical mapping with added keys (e.g., ats, developer-tools, ticketing). Review ordering if UI presentation matters. - Reason this comment was not posted:
Confidence changes required:0%
<= threshold50%
None
Workflow ID: wflow_5e6g5faS4oN6allM
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
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.
👍 Looks good to me! Incremental review on 611b9f7 in 1 minute and 26 seconds
More details
- Looked at
188
lines of code in2
files - Skipped
0
files when reviewing. - Skipped posting
6
drafted comments based on config settings.
1. kits/sdk/openapi.types.d.ts:1
- Draft comment:
This file is auto‐generated. Ensure that any changes in enumerations or definitions reflect intentional updates in the source OpenAPI spec, and avoid manual modifications. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
2. kits/sdk/openapi.types.d.ts:6092
- Draft comment:
Verify that the updated types and enum values are correct and consistent with the new API version to prevent downstream type errors. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
3. kits/sdk/openapi.types.d.ts:1
- Draft comment:
This auto‐generated file has extensive updates. Please double‐check that all updated enum values and endpoint definitions accurately reflect the latest OpenAPI spec. Ensure that the generation process remains consistent with the backend changes. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
4. kits/sdk/openapi.json:448
- Draft comment:
Typographical error: The description for the metadata field contains a duplicated word, 'can can'. Please change it to 'can' to improve clarity. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
5. kits/sdk/openapi.types.d.ts:636
- Draft comment:
Typographical error: Remove the duplicate 'can' in phrases such as 'JSON object can can be used to associate arbitrary metadata'. It should read 'JSON object can be used to associate arbitrary metadata...'. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
6. kits/sdk/openapi.types.d.ts:2817
- Draft comment:
Typographical error: 'implmementation' is misspelled. Please correct it to 'implementation'. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
Workflow ID: wflow_0sjD7UzJf2yq1wrJ
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
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.
lgtm
Important
Memoize
OpenIntConnectEmbed
, update connector verticals, and bump version to 0.2.22.OpenIntConnectEmbed
inembed-react.tsx
for performance improvement.embed-react.tsx
based onparams.theme
.verticals
indef.ts
forconnector-discord
,connector-github
,connector-gong
,connector-intercom
,connector-jira
,connector-kustomer
, andconnector-linear
.package.json
from0.2.19
to0.2.22
.ConnectionsTabContent.tsx
for better UI alignment.This description was created by
for 611b9f7. It will automatically update as commits are pushed.