Skip to content

Private/gokay/doc comp shape#14420

Merged
gokaysatir merged 5 commits intomainfrom
private/gokay/doc-comp-shape
Feb 13, 2026
Merged

Private/gokay/doc comp shape#14420
gokaysatir merged 5 commits intomainfrom
private/gokay/doc-comp-shape

Conversation

@gokaysatir
Copy link
Contributor

  • Resolves: #
  • Target version: main

Summary

TODO

  • ...

Checklist

  • I have run make prettier-write and formatted the code.
  • All commits have Change-Id
  • I have run tests with make check
  • I have issued make run and manually verified that everything looks okay
  • Documentation (manuals or wiki) has been updated or is not required

@caolanm caolanm force-pushed the private/gokay/doc-comp-shape branch from 41bac48 to 01f3cd9 Compare February 11, 2026 20:11
@gokaysatir gokaysatir force-pushed the private/gokay/doc-comp-shape branch 2 times, most recently from a38dfef to be753b1 Compare February 12, 2026 13:37
@gokaysatir gokaysatir requested a review from vmiklos February 13, 2026 05:55
Copy link
Contributor

@vmiklos vmiklos left a comment

Choose a reason for hiding this comment

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

Looks OK in general, I see that dragging shapes in the doc compare view on the RHS is now better; we used to show the preview on the LHS, which was buggy. Please next time describe in the commit messages what are the steps to reproduce the problem you fix, so it's easier to understand your changes, especially if they introduce regressions in the future -- then we'll need to know how to improve the code so both a new and an old use-case works as the same time. So far none of the 4 commit messages contain such description.

Some other comments:

  • svgPosition now contains view coordinates, which made me test the shape anchor overlay, which seems to be buggy. I notice it was bad already before this PR. Seems dragging the shape anchor in Writer now moves the shape? That sounds unexpected.

  • that double anchor counting (see inline) comment looks wrong, but I may miss something.

Could you please go through the comments and either address them or comment there in case I misunderstood something? (And then feel free to merge this yourself.) Thanks.

@github-project-automation github-project-automation bot moved this from To Review to To Test in Collabora Online Feb 13, 2026
Issue: We have new way of coordinate calculation. We now have ViewLayout class and it
can decide to manipulate a point's screen coordinate.
For this use case, we need to use vX, vY properties.

This change fixes mis-located shapes in CompareChanges view layout.

Signed-off-by: Gökay Şatır <gokaysatir@collabora.com>
Change-Id: Ia86285de8581dc2effd9d773d8f38c82a43e6eac
Signed-off-by: Gökay Şatır <gokaysatir@collabora.com>
Change-Id: I87756178c884a5ac2ee6ac414079cb5b6ee06214
Tested on different views and apps with different browser zoom level.

Signed-off-by: Gökay Şatır <gokaysatir@collabora.com>
Change-Id: I7070130b230129423af59661eccc2415bcdd1136
Signed-off-by: Gökay Şatır <gokaysatir@collabora.com>
Change-Id: Id077ed178fb7171444a05a16d327a456b8ba107f
+some misc changes.

Signed-off-by: Gökay Şatır <gokaysatir@collabora.com>
Change-Id: I482fb735630dd4d03362594d7be68de0f3a52a4b
@gokaysatir gokaysatir force-pushed the private/gokay/doc-comp-shape branch from be753b1 to 1bdef21 Compare February 13, 2026 10:46
@gokaysatir
Copy link
Contributor Author

Could you please go through the comments and either address them or comment there in case I misunderstood something? (And then feel free to merge this yourself.) Thanks.

Thanks for checking @vmiklos. I reworded the first commit, added comments where needed and added a condition for the anchor section. Prepared a final commit for changes.

@gokaysatir gokaysatir merged commit cce356a into main Feb 13, 2026
15 checks passed
@gokaysatir gokaysatir deleted the private/gokay/doc-comp-shape branch February 13, 2026 11:24
@github-project-automation github-project-automation bot moved this from To Test to Done in Collabora Online Feb 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants