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

Use a section and remove latlng and leaflet usage from impress&draw c… #10921

Merged
merged 3 commits into from
Jan 20, 2025

Conversation

gokaysatir
Copy link
Contributor

…omments.

Change-Id: Id83eb01940d345b9954c441dd7cb0cff12516fa7

  • Resolves: #
  • Target version: master

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

@gokaysatir gokaysatir force-pushed the private/gokay/latlng-4 branch from a400bca to cbe5e98 Compare January 13, 2025 16:34
@gokaysatir gokaysatir requested review from eszkadev and lpranam and removed request for eszkadev January 13, 2025 16:43
@gokaysatir gokaysatir force-pushed the private/gokay/latlng-4 branch 2 times, most recently from 6c6fa05 to 6b00b1f Compare January 14, 2025 14:59
@gokaysatir
Copy link
Contributor Author

Tests passed on local.

…omments.

Remove unused functions.

Signed-off-by: Gökay Şatır <[email protected]>
Change-Id: Id83eb01940d345b9954c441dd7cb0cff12516fa7
Don't use a section without adding it to the section container.
Use app.map._docLayer instead of the local pointer.

Signed-off-by: Gökay Şatır <[email protected]>
Change-Id: I2b6cd88208677634004f50c5640318472b226f17
…rker.

Signed-off-by: Gökay Şatır <[email protected]>
Change-Id: I089b0d26164773ef752a2c106dc8295a737745b4
@@ -2375,7 +2375,7 @@ export class CommentSection extends app.definitions.canvasSectionObject {
if (this.sectionProperties.docLayer._docType === 'spreadsheet')
this.hideAllComments(); // Apply drawing orders.

if (!(<any>window).mode.isMobile() && (this.sectionProperties.docLayer._docType === 'presentation' || this.sectionProperties.docLayer._docType === 'drawing'))
Copy link
Contributor

Choose a reason for hiding this comment

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

shouldn't be still non-mobile only? on mobile we have annotation mobile-wizard to manage comments

@@ -119,7 +119,7 @@ export class CommentSection extends app.definitions.canvasSectionObject {
this.sectionProperties.showSelectedBigger = false;
this.sectionProperties.calcCurrentComment = null; // We don't automatically show a Calc comment when cursor is on its cell. But we remember it to show if user presses Alt+C keys.
// This (commentsAreListed) variable means that comments are shown as a list on the right side of the document.
this.sectionProperties.commentsAreListed = (this.sectionProperties.docLayer._docType === 'text' || this.sectionProperties.docLayer._docType === 'presentation' || this.sectionProperties.docLayer._docType === 'drawing') && !(<any>window).mode.isMobile();
this.sectionProperties.commentsAreListed = (app.map._docLayer._docType === 'text' || app.map._docLayer._docType === 'presentation' || app.map._docLayer._docType === 'drawing') && !(<any>window).mode.isMobile();
Copy link
Contributor

Choose a reason for hiding this comment

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

does the app.map contain now a correct refrerence? in the past I always saw null there

Copy link
Contributor

@eszkadev eszkadev left a comment

Choose a reason for hiding this comment

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

looks good

@gokaysatir gokaysatir merged commit 09c6935 into master Jan 20, 2025
14 checks passed
@gokaysatir gokaysatir deleted the private/gokay/latlng-4 branch January 20, 2025 11:18
@eszkadev
Copy link
Contributor

I added my UI/UX comment for future: #10964

@gokaysatir
Copy link
Contributor Author

Thanks for checking @lpranam and @eszkadev :)

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.

2 participants