Skip to content
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

we don't need to round-trip through core to reposition notes #7546

Merged
merged 1 commit into from Nov 2, 2023

Conversation

caolanm
Copy link
Contributor

@caolanm caolanm commented Oct 27, 2023

we know where they are by cell addresses, so defer getting the screen positions until we need them and we don't need to get core to trigger recalculating them. When we redraw the comments after the new geometry arrives then we can place them via the address.

When adding a new note we want to know the range of the potentially merged cells we are inserting into.

#7334

https://gerrit.libreoffice.org/c/core/+/158560 needs to be applied to solve the problem described there that becomes apparent when this is in place.

Change-Id: I3228dc8fa8d47ba4e796e50427c125d7f78fe5fc

  • Resolves: #
  • Target version: master

Summary

TODO

  • ...

Checklist

  • Code is properly formatted
  • 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/caolanm/drop_unneeded_comment_refetches branch from 4e37223 to 0a66d7c Compare October 28, 2023 11:10
we know where they are by cell addresses, so defer getting
the screen positions until we need them and we don't need
to get core to trigger recalculating them. When we redraw
the comments after the new geometry arrives then we can place
them via the address.

When adding a new note we want to know the range of the
potentially merged cells we are inserting into.

#7334

https://gerrit.libreoffice.org/c/core/+/158560 needs to be
applied to solve the problem described there that becomes
apparent when this is in place.

Signed-off-by: Caolán McNamara <caolan.mcnamara@collabora.com>
Change-Id: I3228dc8fa8d47ba4e796e50427c125d7f78fe5fc
@caolanm caolanm force-pushed the private/caolanm/drop_unneeded_comment_refetches branch from 0a66d7c to 6f38417 Compare October 29, 2023 09:49
@caolanm caolanm requested a review from mmeeks October 29, 2023 12:09
Copy link
Contributor

@mmeeks mmeeks left a comment

Choose a reason for hiding this comment

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

Looks fine to me; and we're robust over inserting / removing columns, and expanding/contracting row&column grouping and so on I assume ? =)

@caolanm
Copy link
Contributor Author

caolanm commented Oct 31, 2023

grouping/ungrouping looks ok, inserting rows and cols not so much. So https://gerrit.libreoffice.org/c/core/+/158718 to tell the client when a note has moved to another cell

@caolanm caolanm merged commit 72b2ce4 into master Nov 2, 2023
12 checks passed
@caolanm caolanm deleted the private/caolanm/drop_unneeded_comment_refetches branch November 2, 2023 12:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

2 participants