Skip to content

Set new div className to relative instead of absolute #1639

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

Merged
merged 1 commit into from
Apr 4, 2025

Conversation

SoloDevAbu
Copy link
Contributor

@SoloDevAbu SoloDevAbu commented Mar 19, 2025

Description

fixed the issue: When a new div is dragged on the canvas, set it to relative instead of absolute.

Related Issues

fixes#1614

Type of Change

Changed the Tailwind CSS property of absolute to relative

  • [x ] Bug fix
  • New feature
  • Documentation update
  • Release
  • Refactor
  • Other (please describe):

Important

Change position from absolute to relative for new divs in InsertManager.getDefaultProperties() in index.ts.

  • Behavior:
    • Change position from absolute to relative for new div elements in InsertManager.getDefaultProperties() in index.ts.
    • Affects divs dragged onto the canvas, ensuring they are positioned relative to their container.

This description was created by Ellipsis for 2a59a78. It will automatically update as commits are pushed.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❌ Changes requested. Reviewed everything up to 2a59a78 in 1 minute and 55 seconds

More details
  • Looked at 13 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. apps/studio/src/lib/editor/engine/insert/index.ts:43
  • Draft comment:
    Changing to 'relative' looks correct in getDefaultProperties, but note that createInsertAction still hardcodes the div's position as 'absolute'. Ensure consistency across insertion methods.
  • Reason this comment was not posted:
    Marked as duplicate.

Workflow ID: wflow_YsXH74gRT1Lnb3xu


Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

@@ -41,7 +41,7 @@ export class InsertManager {
width: '100px',
height: '100px',
backgroundColor: colors.blue[100],
position: 'absolute',
position: 'relative',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The default div style now uses 'relative'. Verify consistency with other div insertions, e.g., in createInsertAction (line 178) still sets position to 'absolute'.

Suggested change
position: 'relative',
position: 'absolute',

@Kitenite
Copy link
Contributor

@SoloDevAbu thanks for picking this up, perhaps we should go absolute vs relative based on whether parent is flex or grid?
#1614 (comment)

@Kitenite Kitenite merged commit 3129f15 into onlook-dev:main Apr 4, 2025
ml-delaurier pushed a commit to ml-delaurier/nolook that referenced this pull request Apr 23, 2025
t1c1 pushed a commit to t1c1/onlookbotcodes that referenced this pull request Jun 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants