-
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
Fix size of embed loader, hide search on sm devices and improve theme update #278
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 4530d9a in 1 minute and 41 seconds
More details
- Looked at
161
lines of code in5
files - Skipped
0
files when reviewing. - Skipped posting
13
drafted comments based on config settings.
1. apps/connect-dev/.env.example:1
- Draft comment:
Rename token variable to public env var. This change exposes the token in the browser as intended. - Reason this comment was not posted:
Confidence changes required:0%
<= threshold50%
None
2. kits/connect/src/embed-react.tsx:45
- Draft comment:
Use DEFAULT_WIDTH as fallback for iframe width for consistency. Spinner container now also constrains dimensions. - Reason this comment was not posted:
Comment looked like it was already resolved.
3. packages/engine-frontend/components/ConnectionPortal.tsx:56
- Draft comment:
Removed destructured 'theme' from useTheme. Ensure that dark mode styling is handled by a parent component if needed, since the conditional dark class has been removed. - Reason this comment was not posted:
Comment looked like it was already resolved.
4. packages/engine-frontend/components/IntegrationSearch.tsx:134
- Draft comment:
Hide search header on small devices when integrations are few. The logic conditionally adds 'hidden md:block'. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%
<= threshold50%
This comment is purely informative, describing what the code does without suggesting any changes or asking for clarification. It doesn't provide any actionable feedback or raise any concerns about the code.
5. apps/connect-dev/.env.example:1
- Draft comment:
Changed env var name to NEXT_PUBLIC_OPENINT_TOKEN for proper public exposure. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%
<= threshold50%
This comment is purely informative, stating that an environment variable name was changed for public exposure. It doesn't provide a suggestion, ask for confirmation, or highlight a potential issue.
6. apps/connect-dev/app/page.tsx:41
- Draft comment:
Consider extracting inline mouse event styles (onMouseOver/onMouseOut) into CSS classes for maintainability. - Reason this comment was not posted:
Confidence changes required:33%
<= threshold50%
None
7. kits/connect/src/embed-react.tsx:47
- Draft comment:
Using DEFAULT_WIDTH instead of '100%' ensures consistent sizing; confirm responsiveness meets design needs. - Reason this comment was not posted:
Comment looked like it was already resolved.
8. kits/connect/src/embed-react.tsx:96
- Draft comment:
Added max-width and max-height to spinner container to enforce fixed embed dimensions. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%
<= threshold50%
This comment is purely informative, describing what was done without providing any actionable feedback or suggestions. It doesn't ask for confirmation or suggest improvements.
9. kits/connect/src/embed-react.tsx:105
- Draft comment:
Reduced spinner size from 50px to 30px for a more compact embed loader. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%
<= threshold50%
This comment is purely informative, describing a change that was made without providing any actionable feedback or suggestions. It doesn't ask for confirmation or suggest improvements.
10. packages/engine-frontend/components/ConnectionPortal.tsx:56
- Draft comment:
Removed 'theme' from useTheme destructuring; ensure dark mode styling is managed appropriately via the provider. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%
<= threshold50%
The comment is asking the author to ensure that dark mode styling is managed appropriately, which falls under the category of asking the author to ensure behavior is intended or tested. This violates the rules.
11. packages/engine-frontend/components/ConnectionPortal.tsx:66
- Draft comment:
Simplified theme update logic by removing the redundant check against the current theme; verify this aligns with expected behavior. - 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 if the change aligns with expected behavior, which violates the rule against asking for confirmation of intention or behavior. It does not provide a specific code suggestion or ask for a specific test to be written.
12. packages/engine-frontend/components/ConnectionPortal.tsx:159
- Draft comment:
Removed manual dark class toggle from container; ensure dark mode styles are applied via the theme provider. - Reason this comment was not posted:
Comment looked like it was already resolved.
13. packages/engine-frontend/components/IntegrationSearch.tsx:134
- Draft comment:
Conditional class hides search header on small devices when integrations count is below 8; confirm this meets UX requirements. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%
<= threshold50%
The comment is asking for confirmation on whether the behavior meets UX requirements, which is against the rules. It doesn't provide a specific code suggestion or point out a clear issue with the code.
Workflow ID: wflow_7cweeBIYkWkTTHQu
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
import {OpenIntConnectEmbed} from '../../../kits/connect/src/embed-react' | ||
|
||
export default function Home() { | ||
const [theme, setTheme] = useState<'light' | 'dark'>('light') | ||
const token = process.env['NEXT_PUBLIC_OPENINT_TOKEN'] |
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.
No error handling or validation for missing environment variable. Should check if token exists and handle the undefined case to avoid runtime errors.
💬 Reply with
/praise
to tell me that this comment was useful.
Is this comment irrelevant to your project? Reply with
/ignore
to no longer receive comments like this in the future.
😱 Found 1 issue. Time to roll up your sleeves! 😱 |
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 3baf10e in 38 seconds
More details
- Looked at
12
lines of code in1
files - Skipped
0
files when reviewing. - Skipped posting
2
drafted comments based on config settings.
1. kits/connect/package.json:3
- Draft comment:
Version bump update is valid. Ensure corresponding release notes are updated. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%
<= threshold50%
This comment is asking the PR author to ensure that release notes are updated, which falls under the category of asking the author to ensure something is done. This violates the rule against asking the author to ensure things are done.
2. kits/connect/package.json:3
- Draft comment:
Ensure the semantic version bump is appropriate for the changes. Bumping from 0.2.18 to 0.2.19 seems correct if these are patch fixes. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%
<= threshold50%
The comment is asking the author to verify the semantic version bump, which is not allowed as it falls under asking the author to confirm or verify something. The comment does not provide a specific suggestion or point out a clear issue with the version bump, making it not useful according to the rules.
Workflow ID: wflow_0f7lkrmcemnYgf0v
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
Important
Improved UI with theme toggle, adjusted embed loader size, and hid search on small devices, along with minor environment and package updates.
page.tsx
: Added theme toggle button with light/dark mode functionality.embed-react.tsx
: Adjusted spinner size to 30px and set max-width/height for iframe.IntegrationSearch.tsx
: Hid search bar on small devices when less than 8 integrations.OPENINT_TOKEN
toNEXT_PUBLIC_OPENINT_TOKEN
in.env.example
.package.json
from0.2.18
to0.2.19
.ConnectionPortal.tsx
: Removed redundant theme check before setting theme.This description was created by
for 3baf10e. It will automatically update as commits are pushed.