Bug 393811 - Inconsistent behaviour in saving comments
Summary: Inconsistent behaviour in saving comments
Status: REPORTED
Alias: None
Product: okular
Classification: Applications
Component: general (show other bugs)
Version: 1.3.1
Platform: Ubuntu Linux
: NOR normal
Target Milestone: ---
Assignee: Okular developers
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-05-03 17:00 UTC by david08741
Modified: 2022-03-01 22:42 UTC (History)
3 users (show)

See Also:
Latest Commit:
Version Fixed In:
Sentry Crash Report:


Attachments
selecting note shape/area (114.04 KB, image/png)
2022-03-01 19:54 UTC, mineirodomundo
Details
note created and note editor opened (105.88 KB, image/png)
2022-03-01 19:55 UTC, mineirodomundo
Details
text entry using note editor (114.48 KB, image/png)
2022-03-01 19:56 UTC, mineirodomundo
Details
note with text (100.10 KB, image/png)
2022-03-01 19:57 UTC, mineirodomundo
Details
selecting area for second note (415.12 KB, image/png)
2022-03-01 20:13 UTC, mineirodomundo
Details
current note with unexpected size (399.06 KB, image/png)
2022-03-01 20:14 UTC, mineirodomundo
Details
expected result of the second note (402.74 KB, image/png)
2022-03-01 20:16 UTC, mineirodomundo
Details

Note You need to log in before you can comment on or make changes to this bug.
Description david08741 2018-05-03 17:00:41 UTC
Description of problem:
If hitting Escape while entering a comment, it does depend on the specific comment on whether the comment is saved or deleted (without asking)

Version-Release number of selected component (if applicable):
okular-17.12.1-1.fc27.x86_64

okular --version
okular 1.3.1

How reproducible:
Always

Steps to Reproduce:
1. Open Pop-up note [1]
2. Type
3. Press Esc
4. Open Inline Note [2]
5. Type
6. Press Esc

Actual results:
Only Pop up note is saved, inline note is gone

Expected results:
Both should be there

Additional info:
Comment 1 ederag 2018-08-02 15:30:58 UTC
confirmed, in okular 1.3.3
Comment 2 Albert Astals Cid 2018-08-08 22:00:38 UTC
When you say "Open" you mean "Create", right?

If so, i disagree this is inconsistent.

When creating a Popup note, you place the popup not and then the enter text dialog is open, that dialog doesn't have cancel

When creating an inline note, you get a dialog that says "Enter text for new note", and it has a Cancel button, so Esc presses that cancel button.
Comment 3 david08741 2018-08-23 21:00:15 UTC
Most likely, but as it is not saved, I am not sure "Create" is the right word, at least for the inline note.

I think that Esc triggers the Cancel button may be an explanation, of why this is happening, however, I still think this behaviour is very annoying, as if the notes are edited, pressing Esc saves the changes and closes the notes, thus it is a very convenient button, to save the changes and close the note.

In order to make the notes more consistent, could the inline note also be created the same way as the popup note, and the window for editing be opened?
That way, no `Cancel' button would be present, that would be triggered by Esc?
Comment 4 Albert Astals Cid 2018-08-23 21:15:47 UTC
I'm sorry, but you should not confirm your own bugs, that's like basically ignoring my disagreement and going for "i'm right".

About as if the editing for Inline notes could be done as like the ones for Popup, i don't think that would make sense, they are different things and thus they are created in a way that makes sense for each of them.
Comment 5 david08741 2018-08-23 21:26:39 UTC
Sorry, I wanted to change it back from information required, and the last state was confirmed (I think) - I had no intention in saying you are wrong. I thought confirmed meant that the behaviour has been confirmed (as was in comment 1), not that it is a bug. This will not happen again. Would the appropriate thing be to leave it as is (e.g. needsinfo) or change it? I assume to unconfirmed?

If the Cancel button is preferred, could a check be added, to see if the written text really should be deleted?
I would also be happy if it could be added to the history of changes, such that Ctrl-Z would bring the text back ...

As a work around I started to first create the inline notes, save, and only then enter the text - but I occasionally start typing to soon, and then my work is gone.
Comment 6 Albert Astals Cid 2018-08-23 21:45:00 UTC
(In reply to david08741 from comment #5)
> Sorry, I wanted to change it back from information required, and the last
> state was confirmed (I think) - I had no intention in saying you are wrong.
> I thought confirmed meant that the behaviour has been confirmed (as was in
> comment 1), not that it is a bug. This will not happen again. Would the
> appropriate thing be to leave it as is (e.g. needsinfo) or change it? I
> assume to unconfirmed?

Ok, don't worry, we all make mistakes sometimes :)

Yes, Unconfirmed would have been better, but it may be true that given the state it was it would not let you choose it.

> If the Cancel button is preferred, could a check be added, to see if the
> written text really should be deleted?

Are you asking for a "Do you really want to Cancel" messagebox? 

> As a work around I started to first create the inline notes, save, and only
> then enter the text - but I occasionally start typing to soon, and then my
> work is gone.

I don't understand why your text is gone, why are you pressing Esc all the time?
Comment 7 david08741 2018-08-23 21:58:05 UTC
Yes, "Do you really want to Cancel" would be nice.

After I edited a note, I want to save the changes and close the window, I press Esc, as that key does just that - saving and closing. (With the exception of freshly created inline notes).
If there is a better key, that saves and closes the window, I have not found it.

If the note is however a freshly created inline note, I press Esc because it works most of the time, but then the text is gone.
Comment 8 mineirodomundo 2022-03-01 14:15:03 UTC
Hello everyone,

I tend to agree David, so I would like to raise this issue again.
It seems inconsistent Okular adds new inline and pop-up notes.

In particular, I like that I am able to add a new pop-up note without having to click on a button, just pressing 'esc' after I finished typing the note.
However, if I am creating an in-line note I always need to click on the OK button, since there is no shortcut to confirm the text.

I did some digging end found how this is done:
https://invent.kde.org/graphics/okular/-/blob/master/part/pageviewannotator.cpp#L203

I wonder if I could work on this changes and prepare a pull request.

Cheers,
Comment 9 Albert Astals Cid 2022-03-01 15:28:20 UTC
Maybe I'm not understanding what is the problem, but I'm not convinced what we do is wrong, creating a new annotation and editing an existing one are  two different things.

They do have different user interfaces too, one is a normal dialog with "Ok and Cancel" buttons and the other is the "fancy, this looks like a post-it" editor.

So they behave differently.

Anyhow, yes you can prepare a Merge Request, but I would prefer we agree on what you're going to do first, don't want you to work on something that then will be rejected.

Can you try to explain what you'd like to change?
Comment 10 mineirodomundo 2022-03-01 19:54:39 UTC
Created attachment 147220 [details]
selecting note shape/area
Comment 11 mineirodomundo 2022-03-01 19:55:51 UTC
Created attachment 147221 [details]
note created and note editor opened
Comment 12 mineirodomundo 2022-03-01 19:56:29 UTC
Created attachment 147222 [details]
text entry using note editor
Comment 13 mineirodomundo 2022-03-01 19:57:13 UTC
Created attachment 147223 [details]
note with text
Comment 14 mineirodomundo 2022-03-01 20:13:11 UTC
Created attachment 147225 [details]
selecting area for second note
Comment 15 mineirodomundo 2022-03-01 20:14:35 UTC
Created attachment 147226 [details]
current note with unexpected size
Comment 16 mineirodomundo 2022-03-01 20:16:31 UTC
Created attachment 147227 [details]
expected result of the second note
Comment 17 mineirodomundo 2022-03-01 20:26:06 UTC
Hi Albert,

Thanks for the quick reply.

I plan to have the following workflow:

1- select the region 
2- once the mouse is released
  2.1- create the note
  2.2- open inline note editor
3- the user can enter the text like when it is editing an inline note 
4- the user might close the editor using ESC

I attached some screenshots that I hope will give a better idea of this workflow.

An effect of this change would be that the area initially chosen by the user would define the size of the note.
Currently, Okular readjusts the area of the note depending on the size of the text entered, but this might lead the note to hide something that might be important for the user.
In the example attached, the idea is that the user could define the note by the side of the title, which currently covers the title, in the new workflow that would not happen.

Thanks for the hard work. Okular is my favorite way of taking notes.
Cheers,
Comment 18 Albert Astals Cid 2022-03-01 22:42:43 UTC
So you would skip the creation phase ...

... i guess that would be ok? The other annotations don't have that phase either, similar to what the Popup Note does