-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Fix edit panel not fully hidden #1695
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
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 a504493 in 1 minute and 19 seconds
More details
- Looked at
26
lines of code in1
files - Skipped
0
files when reviewing. - Skipped posting
4
drafted comments based on config settings.
1. apps/studio/src/routes/editor/EditPanel/index.tsx:223
- Draft comment:
Consider using an empty string ('') instead of 'visible' when not in PREVIEW mode, since Tailwind's default is visible. This would clarify intent. - Reason this comment was not posted:
Confidence changes required:33%
<= threshold50%
None
2. apps/studio/src/routes/editor/EditPanel/index.tsx:239
- Draft comment:
Removal of duplicate conditional class on the dev-panel div appears correct. Confirm that this change fully hides the panel in PREVIEW mode. - Reason this comment was not posted:
Confidence changes required:0%
<= threshold50%
None
3. apps/studio/src/routes/editor/EditPanel/index.tsx:223
- Draft comment:
Using the editorEngine.mode === EditorMode.PREVIEW condition on the outer container now hides the entire edit panel as intended. Consider if explicitly adding 'visible' is necessary, since the default state is visible. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50%
The comment is about code that was changed in the diff. The suggestion is technically correct - 'visible' is indeed the default state and doesn't need to be explicitly specified. However, being explicit about both states in a ternary can make the code more readable and maintainable. The change is very minor and more of a style preference.
The explicit 'visible' state might be intentional for code clarity and consistency with other similar toggles in the file. The change is quite minor and could be considered too nitpicky.
While being explicit with 'visible' is redundant, it's a valid stylistic choice that makes the code's intent clearer and matches the pattern used elsewhere in the file.
This comment should be deleted as it suggests a minor stylistic change that doesn't meaningfully improve the code quality.
4. apps/studio/src/routes/editor/EditPanel/index.tsx:265
- Draft comment:
The conditional class 'editorEngine.mode === EditorMode.PREVIEW ? "hidden" : "visible"' on the 'style-panel' div might be redundant given the outer container already applies the same logic. If not required for additional styling transitions, consider removing it to avoid duplication. - 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_hMhCZo0uYHPV8yon
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 2d22d80 in 36 seconds
More details
- Looked at
15
lines of code in1
files - Skipped
0
files when reviewing. - Skipped posting
3
drafted comments based on config settings.
1. apps/studio/src/routes/editor/EditPanel/index.tsx:238
- Draft comment:
The redundant use of the cn() helper has been removed. Confirm that no conditional logic is required for the 'dev-panel' class. - Reason this comment was not posted:
Confidence changes required:0%
<= threshold50%
None
2. apps/studio/src/routes/editor/EditPanel/index.tsx:238
- Draft comment:
Using a static string for the dev-panel's className is fine if no conditional classes are needed. In case dynamic conditions are intended later, consider continuing to use the cn() utility. - Reason this comment was not posted:
Confidence changes required:33%
<= threshold50%
None
3. apps/studio/src/routes/editor/EditPanel/index.tsx:1
- Draft comment:
The PR title and diff indicate a fix for the edit panel’s visibility, but the issue details mention onboarding docs. Please update the PR description to clearly reflect the changes being made. - 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_6p7QrGTPU8owZagL
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
…slation_zh * 'main' of https://github.com/onlook-dev/onlook: Publish version v0.2.23 (onlook-dev#1694) Fix edit panel not fully hidden (onlook-dev#1695) Update file watcher and remove elide lines (onlook-dev#1693) Add sticky position (onlook-dev#1692) Publish version v0.2.22 (onlook-dev#1679) added text transforms (onlook-dev#1689) Update URL when changing project settings URL (onlook-dev#1690) fix: error when setting text color (onlook-dev#1688) Revert freestyle source (onlook-dev#1686)
Description
Related Issues
Type of Change
Testing
Screenshots (if applicable)
Additional Notes
Important
Fixes visibility issue in
EditPanel
by hiding it in preview mode using a conditional class.EditPanel
inindex.tsx
by adding conditional class to hide panel inEditorMode.PREVIEW
.dev-panel
div.This description was created by
for 2d22d80. It will automatically update as commits are pushed.