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

Fix toggle status of show all comments button #10803

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

GulsahKose
Copy link
Contributor

@GulsahKose GulsahKose commented Dec 25, 2024

Before initial state was always true.
Now we decide to initial state according to document have comments or not. If document have comments, default initial state is comments on, if not comments off.

Change-Id: If3e8eb5eeff29c8a72658d0afafc7379444d62ab

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

Before initial state was always true.
Now we decide to initial state according to document have comments or
not. If document have comments, default initial state is comments on, if
not comments off.

Signed-off-by: Gülşah Köse <[email protected]>
Change-Id: If3e8eb5eeff29c8a72658d0afafc7379444d62ab
@GulsahKose GulsahKose requested a review from eszkadev December 26, 2024 10:10
var show = this.map.stateChangeHandler.getItemValue('showannotations');
this.setView(show === true || show === 'true');
var initalShowAnnotations = commentList.length > 0;
app.map.showComments(initalShowAnnotations);
Copy link
Contributor

Choose a reason for hiding this comment

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

This makes it out of sync when inserting a comment to document which didn't have comments before.
Then you have to click 2 times "show comments" to acctually toggle.

I think it should stay as it was (default is on) - less problems.

As improvement we can probably remember the last state which user had in the local browser settings? cc: @pedropintosilva
Or maybe it is saved in the document? Could you try with odt and docx, if they remember the last status after we save and reopen document in LibreOffice?

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.

comment above

@eszkadev eszkadev added the draft label Dec 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: In Progress
Development

Successfully merging this pull request may close these issues.

Show/Hide comments toggle resets resolved comments toggle
2 participants