Correctly set underMouse() for inline notes
ClosedPublic

Authored by davidre on Jan 22 2020, 1:14 PM.

Details

Summary

Before it was always false for every inline note.

Test Plan

Diff Detail

Repository
R39 KTextEditor
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
davidre created this revision.Jan 22 2020, 1:14 PM
Restricted Application added projects: Kate, Frameworks. · View Herald TranscriptJan 22 2020, 1:14 PM
Restricted Application added subscribers: kde-frameworks-devel, kwrite-devel. · View Herald Transcript
davidre requested review of this revision.Jan 22 2020, 1:14 PM
davidre edited the test plan for this revision. (Show Details)Jan 22 2020, 1:16 PM
davidre added a reviewer: KTextEditor.
cullmann accepted this revision.Jan 22 2020, 4:20 PM
cullmann added a subscriber: cullmann.

Looks reasonable.
Btw., does somebody use that now for something useful?

This revision is now accepted and ready to land.Jan 22 2020, 4:20 PM
brauch added a subscriber: brauch.Jan 22 2020, 6:05 PM

Is the video the new behaviour? It still looks a bit weird to me, there is a slight delay between the mouse entering the area and the highlight changing.

Is it possible that the line (and thus the note) is not properly marked for repaint when the underMouse property changes?

Is the video the new behaviour? It still looks a bit weird to me, there is a slight delay between the mouse entering the area and the highlight changing.

Is it possible that the line (and thus the note) is not properly marked for repaint when the underMouse property changes?

Yes it's the new behavior, before it wouldn't change at all. I think the tagLines marks the line for repainting but I didn't yet look if the this also triggers painting of lines. Or maybe it's a bit slow because of my debug setup?

Hm, yeah, looking at the code I think you might need to call updateView() in case a focus in or out happened. Can you try if that makes a difference?

My guess is right now the view updates when the cursor blinks, so that's why it updates after a short moment (of varying length, though, if you look at the video). Since the line is tagged dirty, it gets repainted correctly, but too late.

My guess is right now the view updates when the cursor blinks, so that's why it updates after a short moment (of varying length, though, if you look at the video). Since the line is tagged dirty, it gets repainted correctly, but too late.


It seems you're right but updateView doesn't help. The video above is with updateView() calls and slowed down.

Then it seems like the line is not tagged correctly. Maybe try tagLines(note.position.line(), note.position.line())? I think the column being the same is not what the function being called expects.

Then it seems like the line is not tagged correctly. Maybe try tagLines(note.position.line(), note.position.line())? I think the column being the same is not what the function being called expects.

Hmm doesn't seem to be instantly either. But I have the feeling it's worse when I try to record it so it's maybe an issue on my laptop. Would you object if I land this then, given this is more correct than before?

I'm sorry, updateView is the wrong function to call, you need updateDirty. I tried it out, like this it works:

if (e->buttons() == Qt::NoButton) {
    auto noteData = inlineNoteAt(e->globalPos());
    if (noteData.m_position.isValid()) {
        if (!m_activeInlineNote.m_position.isValid()) {
            // no active note -- focus in
            tagLine(noteData.m_position);
            updateDirty();
            noteData.m_underMouse = true;
            noteData.m_provider->inlineNoteFocusInEvent(KTextEditor::InlineNote(noteData), e->globalPos());
            m_activeInlineNote = noteData;
        } else {
            noteData.m_provider->inlineNoteMouseMoveEvent(KTextEditor::InlineNote(noteData), e->globalPos());
        }
    } else if (m_activeInlineNote.m_position.isValid()) {
        tagLine(m_activeInlineNote.m_position);
        updateDirty();
        m_activeInlineNote.m_underMouse = false;
        m_activeInlineNote.m_provider->inlineNoteFocusOutEvent(KTextEditor::InlineNote(m_activeInlineNote));
        m_activeInlineNote = {};
    }
}
brauch accepted this revision.Jan 25 2020, 8:27 AM

If you want I can integrate these changes and submit your patch, should I? Thanks a lot for your contribution.

If you want I can integrate these changes and submit your patch, should I? Thanks a lot for your contribution.

Feel free to!

Ok, done ;)

This revision was automatically updated to reflect the committed changes.