feat(): Extended New Snapping Guidelines for objects #8955

CODE-AXION · 2023-05-24T04:52:26Z

1) The feature was added using the existing code methods mentioned in aligning_guidelines.js file

New Features I Added In Snapping Guidelines:

// snaps if the top of the active object touches the bottom of the other object
// snaps if the bottom of the active object touches the top of the other object
// snaps if the left side of the active object touches the right side of the other object
// snaps if the right side of the active object touches the left side of the other object

Existing Snapping Guidelines:

snap by the horizontal center line
snap by the left edge
snap by the right edge
snap by the vertical center line
snap by the top edge
snap by the bottom edge

New Snapping Guidelines screenshots:
snaps if the left side of the active object touches the right side of the other object:

snaps if the right side of the active object touches the left side of the other object

snaps if the top of the active object touches the bottom of the other object:

snaps if the bottom of the active object touches the top of the other object:

2) Fixed a bug

bug: Throws an error when i try to download the canvas in different formats like png , jpg etc..

How i fixed: added condition check on lib/centering_guidelines.js and lib/aligning_guidelines.js

Error:

asturur · 2023-05-24T19:53:23Z

@CODE-AXION thank you for the PR!
Today till monday i will be out and i won't be able to look at it.

I will back to you soon

…e.js also fixed a bug when downloading the canvas to different formats so added some conditions on canvas:before render code part .

CODE-AXION · 2023-06-08T05:10:02Z

@CODE-AXION thank you for the PR! Today till monday i will be out and i won't be able to look at it.

I will back to you soon

@asturur
hey when you will checkout my PR ?

CODE-AXION · 2024-02-27T17:52:59Z

@asturur Hi its been a long time can you review this PR ?

github-actions · 2024-03-12T08:38:30Z

Build Stats

file / KB (diff)	bundled	minified
fabric	909.937 (0)	304.737 (0)

asturur · 2024-03-12T09:04:07Z

Those will need to be updated to TS and exported properly for v6, but i see no harm in merging as is.
I m sorry i couldn't pay attention to you for years but the repo is higly congested and things are a bit stuck and i don't want to work on fabricjs 8 hours a day.

Smrtnyk · 2024-03-12T09:06:24Z

This is the feature I am very interested in
Does this piece of code work also when canvas is zoomed in?

asturur · 2024-03-12T09:07:07Z

This is the feature I am very interested in Does this piece of code work also when canvas is zoomed in?

Not sure, but if it doesn't it should pretty easy to fix.

asturur · 2024-03-12T09:08:22Z

You have also to consider this code hasn't been tested with v6 and more importantly has no tests at all

ShaMan123 · 2024-03-12T09:09:36Z

lib/aligning_guidelines.js

+            : (activeObjectTop - activeObjectHeight / 2 - aligningLineOffset)
+        });
+
+        activeObject.setPositionByOrigin(new fabric.Point(objectLeft - objectWidth / 2 - activeObjectWidth / 2, activeObjectTop), 'center', 'center');


do not use setPositionByOrigin, it is wrong for a nested object

use setXY since the point is in the scene

ShaMan123 · 2024-03-12T09:11:01Z

lib/aligning_guidelines.js

+
+        verticalInTheRange = true;
+        verticalLines.push({
+          x: objectLeft - objectWidth / 2,


objectLeft is a wrong name

ShaMan123 · 2024-03-12T09:11:20Z

lib/aligning_guidelines.js

+    if (canvas.contextTop) {
+      canvas.clearContext(canvas.contextTop);
+    }


not needed if v6

the right approach would be
canvas.contextTopDirty = true

Smrtnyk · 2024-03-12T09:11:51Z

You have also to consider this code hasn't been tested with v6 and more importantly has no tests at all

When this is merged I could help with conversion to typescript, since I need it for my fabric project where I use v6 beta, I have to convert it to typescript anyway to be able to use it.

asturur · 2024-03-12T09:14:30Z

@ShaMan123 This code won't probably work with nested selections inside groups, but also it wasn't in scope at the time of changes.
This whole guidelines code has probably a large number of incompatibilities with v6 and needs to be converted and evaluated again from scratch.

It hasn't been reviewed for ages and i m not going to ask changes now isn't really helpful.
We have to assume it works as is for the author and that anyway we would have to fix guidelines for v6 and typescript and the little amount of code he added makes no difference for us.

asturur · 2024-03-12T09:18:17Z

The real issue with this code is that it assumes that there is an object called 'fabric' that doesn't exist in that context.

asturur · 2024-03-12T11:25:25Z

@Smrtnyk i suggest you do not dive converting this exact code in TS.
There are things to consider

scaling support
exposing this feature in the module world we are now

I hope we can close 6.0 soon and then fit this in a roadmap with some specs, then if you think you can take it on, you can take it on, with specs

asturur · 2024-03-12T11:25:45Z

@CODE-AXION on which version of fabricJS are you using this exact code?

CODE-AXION · 2024-03-12T13:10:28Z

@CODE-AXION on which version of fabricJS are you using this exact code?

@asturur ok so i opened the project on my local and at that time i was using "version": "6.0.0-beta6" (from package.json)

asturur · 2024-03-12T13:26:06Z

So it works for you also if it references fabric.Point? do you have fabric object on the window object?

CODE-AXION · 2024-03-12T14:25:42Z

So it works for you also if it references fabric.Point? do you have fabric object on the window object?

@asturur yes i am using fabric on the window object

asturur · 2024-08-10T20:14:42Z

@CODE-AXION someone refactored all the guidelines in TS and i worked to make them an extension package in fabric.
I never came back to this PR but i merged today because i wanted to leave track of your effort of fixing them with the commit.
I also need to compare both implementation and make a review of both approaches, probably the best will be a mix of the 2

CODE-AXION changed the title ~~Extended New Snapping Guidelines in aligning_guidelines.js~~ feat(): Extended New Snapping Guidelines in aligning_guidelines.js May 24, 2023

added new snapping guidelines on different sides in aligning_guidelin…

8ae87d2

…e.js also fixed a bug when downloading the canvas to different formats so added some conditions on canvas:before render code part .

CODE-AXION force-pushed the feature/aligining-guidelines--additions branch from f89a1d6 to 8ae87d2 Compare May 26, 2023 18:28

CODE-AXION closed this Jun 8, 2023

CODE-AXION reopened this Jun 8, 2023

asturur self-assigned this Jun 12, 2023

This was referenced Jun 12, 2023

fix(ClassRegistry) add the extended names to the registry #9007

Closed

ci(): Refactor GHA for reuse and caching #9029

Merged

Merge branch 'master' into feature/aligining-guidelines--additions

8efa47f

asturur changed the title ~~feat(): Extended New Snapping Guidelines in aligning_guidelines.js~~ feat(): Extended New Snapping Guidelines Mar 12, 2024

Update CHANGELOG.md

f3fdb03

asturur changed the title ~~feat(): Extended New Snapping Guidelines~~ feat(): Extended New Snapping Guidelines for objects Mar 12, 2024

ShaMan123 reviewed Mar 12, 2024

View reviewed changes

asturur approved these changes Mar 12, 2024

View reviewed changes

ShaMan123 reviewed Mar 12, 2024

View reviewed changes

Merge branch 'master' into feature/aligining-guidelines--additions

3524ee2

asturur merged commit e577442 into fabricjs:master Aug 10, 2024
6 of 8 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(): Extended New Snapping Guidelines for objects #8955

feat(): Extended New Snapping Guidelines for objects #8955

CODE-AXION commented May 24, 2023 •

edited

Loading

asturur commented May 24, 2023

CODE-AXION commented Jun 8, 2023 •

edited

Loading

CODE-AXION commented Feb 27, 2024

github-actions bot commented Mar 12, 2024

asturur commented Mar 12, 2024

Smrtnyk commented Mar 12, 2024

asturur commented Mar 12, 2024

asturur commented Mar 12, 2024

ShaMan123 Mar 12, 2024

ShaMan123 Mar 12, 2024

ShaMan123 Mar 12, 2024

ShaMan123 Mar 12, 2024

ShaMan123 Mar 12, 2024

Smrtnyk commented Mar 12, 2024

asturur commented Mar 12, 2024

asturur commented Mar 12, 2024

asturur commented Mar 12, 2024

asturur commented Mar 12, 2024

CODE-AXION commented Mar 12, 2024

asturur commented Mar 12, 2024

CODE-AXION commented Mar 12, 2024 •

edited

Loading

asturur commented Aug 10, 2024

feat(): Extended New Snapping Guidelines for objects #8955

feat(): Extended New Snapping Guidelines for objects #8955

Conversation

CODE-AXION commented May 24, 2023 • edited Loading

1) The feature was added using the existing code methods mentioned in aligning_guidelines.js file

2) Fixed a bug

asturur commented May 24, 2023

CODE-AXION commented Jun 8, 2023 • edited Loading

CODE-AXION commented Feb 27, 2024

github-actions bot commented Mar 12, 2024

asturur commented Mar 12, 2024

Smrtnyk commented Mar 12, 2024

asturur commented Mar 12, 2024

asturur commented Mar 12, 2024

ShaMan123 Mar 12, 2024

Choose a reason for hiding this comment

ShaMan123 Mar 12, 2024

Choose a reason for hiding this comment

ShaMan123 Mar 12, 2024

Choose a reason for hiding this comment

ShaMan123 Mar 12, 2024

Choose a reason for hiding this comment

ShaMan123 Mar 12, 2024

Choose a reason for hiding this comment

Smrtnyk commented Mar 12, 2024

asturur commented Mar 12, 2024

asturur commented Mar 12, 2024

asturur commented Mar 12, 2024

asturur commented Mar 12, 2024

CODE-AXION commented Mar 12, 2024

asturur commented Mar 12, 2024

CODE-AXION commented Mar 12, 2024 • edited Loading

asturur commented Aug 10, 2024

CODE-AXION commented May 24, 2023 •

edited

Loading

CODE-AXION commented Jun 8, 2023 •

edited

Loading

CODE-AXION commented Mar 12, 2024 •

edited

Loading