Bug 158379

Just to be clear, frame focus indications should be done by the style, not the application. So this sounds like either a style problem or an application-style integration problem.

I assume this is from trunk (i.e. KDE4+, not KDE3.x)?

It's indeed a katepart problem: Katepart needs to draw a focus rect primitive. If we do that, we either have to include the scrollbar or not.
If included, it violates fitt's law (or whatever it's called) ;)
If not incled, it maybe looks strange.

I think it's best to include the scrollbar in the focus rect. But I don't see how it is connected to fitt's law, which says just that the mouse movement time is longer for smaller target objects (and also longer for bigger travel distance).

Eclipse does include the scrollbar in the focus rect and I've not noticed any usability problems with it when using eclipse.

PS This wish is for kde4/trunk.

What I mean is, if you maximize the window, you can move the mouse to the very right of the screen and simply click/move the mouse and it works. If we have a focus rect, there are 1-2 more pixel between the scrollbar and the screen edge, so that this does not work anymore.

In KDE3, kwrite works. Kate has already an additional frame from a tabbar, so that it does not work. Well, who cares, maybe we simply should add the frame and be done :)

If you use oxygen, the blue rect is drawn.
If you use other widget styles, it is not drawn.

So it should rather be fixed in Katepart instead of in the widget style.

*** Bug 280638 has been marked as a duplicate of this bug. ***

KDE 4: The focus rect is drawn by the widget style (Oxygen).

KF5: Drawing the focus rect was removed here:
https://projects.kde.org/projects/kde/kde-workspace/repository/revisions/00c61a7f233c6f234acafbf4c2a4f93878e1545b

However, the question is now how Kate Part can have this animated Oxygen effect: The focus rect fades in and out.

@Hugo:
Can this be done on application level correctly at all?
Because if not, the above commit probably needs to be reverted?

To sum up again: Styles have much more control over how a focus rect appears: Oxygen and other styles like Breeze have a decent fade-in and fade-out animation. It's not possible to draw this animation from the client-side. Therefore, the focus rect *has* to be drawn in the style, I don't see a way around that... Therefore, this will as of now not be solved within kate part. Either the styles implement the focus rect, or it's not there.

Not having a focus rect may currently not be a big thing, since the flat design of current themes anyway don't require a that much distinct focus rect. But we'll leave that for the style designers :-)

We'll reassign this to to style components again.

(In reply to Dominik Haumann from comment #8)
> To sum up again: Styles have much more control over how a focus rect
> appears: Oxygen and other styles like Breeze have a decent fade-in and
> fade-out animation. It's not possible to draw this animation from the
> client-side. Therefore, the focus rect *has* to be drawn in the style, I
> don't see a way around that... Therefore, this will as of now not be solved
> within kate part. Either the styles implement the focus rect, or it's not
> there.
> 
> Not having a focus rect may currently not be a big thing, since the flat
> design of current themes anyway don't require a that much distinct focus
> rect. But we'll leave that for the style designers :-)
> 
> We'll reassign this to to style components again.

Hi Dominik,
I 'think' that to have focus indicator, first you need a QFrame in the widget stack, or something that mimics a QFrame to the style, a la 

style()->drawControl(QStyle::CE_ShapedFrame, &opt, p, this);

And make sure that the passed option (&opt) has the right focus bit set (as well as Hover, for that matter). 

That should do the trick for all styles.
Additional tweaking might be needed for oxygen and breeze, to plug in animation on top of that.

Now, the current kate, kwrite look rather flat ! 
No frame -> no focus :D

Personally, I believe whether the view should be rendered flat or not should be left to the style (it will be quite flat-ish in breeze as all the rest, but would look better sunken in oxygen), and that would let them do the rendering. 

 I'm willing to help achieve this in ktexteditor, test, and submit patch to you for review, 
I'd might need some help to get me started though and for instance I'm not sure what widget would be the right one to hold the frame ? 
KTextEditor::View ?

Hi, 
After experimenting a bit, and just to make sure that we are talking about the same thing:

http://wstaw.org/m/2014/09/10/plasma-desktopgs4453.png
http://wstaw.org/m/2014/09/10/plasma-desktopMf4453.png

Is what this is about. Correct ? 
I have a (near final) set of changes to frameworks/ktexteditor/src/kateview that allows to achieve the above. Will submit reviewboard when final. 

Unfortunatly, I could only test with breeze and oxygen, because the other widget styles I know of (fusion), simply don't implement any focus effect on text editors ... 

Also there are changes to be pushed to both oxygen and breeze for this to work (but that is somewhat unrelated, and due to their internals only).

I will try to test with qtcurve as soon as I have it compiled for KF5

Git commit c71ed504dad4f92979e905067b22cafc015b8afd by Hugo Pereira Da Costa.
Committed on 11/09/2014 at 21:18.
Pushed by hpereiradacosta into branch 'master'.

- Use QGridLayout for setting up View layout
- Added QSpacerItems in order to properly implement frame around edition area.
- properly account for whether style requires frame around contents only, or around contents + scrollbars
- Call style primitive in order to render a frame around the said area, with proper mouse-over and focus set.
- Adjust scrollbar background based on whether frame must be drawn around contents only, or scrollbars+contents
- Force update of widget and scrollbars on focusIn and focusOut events
REVIEW: 120147

M  +224  -31   src/view/kateview.cpp
M  +11   -1    src/view/kateview.h

http://commits.kde.org/ktexteditor/c71ed504dad4f92979e905067b22cafc015b8afd

Closing because fixed after commits from previous comments