Merge lp:~tiagosh/unity-2d/fix-dash-cursor into lp:unity-2d

[Tiago Salem Herrmann]

[dash] make the dash cursor blink

[Gerry Boland (gerboland)]

You've changed the appearance of the cursor with this. Cursor should be 2 pixels wide.

review: Needs Fixing

[Gerry Boland (gerboland)]

Please also supply screenshot of the change, so Design can sign this change off

[Gerry Boland (gerboland)]

My apologies, I did not notice that you inserted videos into the attached bug. They're great, thanks.

We just need to get get Design confirmation that 1pixel cursor width is correct, and this is good to go!
Thanks
-G

[Gerry Boland (gerboland)]

<greyback> JohnLea: quickest of questions! Width of flashing cursor in Dash search box is 1 pixel wide?
<JohnLea> greyback; I think so

Over IRC, you also confirmed with me that Unity uses 1 pixel wide cursor, so please withdraw my objections there.

I dug into Nux to see their cursor flash time is 400ms. Be nice to match that.

[Gerry Boland (gerboland)]

Chatting with Rosie, she pointed out that we should use the HUD mockups which have a cursor visible.

Cursor there is 1x30pixels, with 5 pixels margin above & below. Yours is 34pixels high.

review: Needs Fixing (design (rosie))

[Tiago Salem Herrmann (tiagosh)]

Looks like it impossible to change the cursor height:

http://qt.gitorious.org/qt/qt/blobs/4.8/src/declarative/graphicsitems/qdeclarativetextinput.cpp#line988

The height is set according to the font size:

http://qt.gitorious.org/qt/qt/blobs/4.8/src/declarative/graphicsitems/qdeclarativetextinput.cpp#line242

[Gerry Boland (gerboland)]

Can you not make a simple cursor component?

[Tiago Salem Herrmann (tiagosh)]

As discussed on IRC, unfortunately we can't change the height even for custom cursors, and the default one is already 1 px wide. In case we don't really need the fading animation, I think this patch matches the spec.

[Gerry Boland (gerboland)]

Playing with this, the following cursor component works for me:

Rectangle {
    color: "white"
    anchors.top: parent.top
    anchors.bottom: parent.bottom
    anchors.topMargin: 4
    anchors.bottomMargin: 8
    width: 1
....
}

and the above/below margins are adjustable.

[Paul Sladen (sladen)]

The tree here still leaves the text too large. I did manage to get it to the correct size with 'height: height-2'; see:

  lp:~sladen/unity-2d/unity-2d-dash-cursor-size-lp942122

although clearly, getting ride of the margins/padding, then forcing the height upwards would be preferable.

review: Needs Fixing (functionality)

[lp:~tiagosh/unity-2d/fix-dash-cursor]

918. By Tiago Salem Herrmann on 2012-02-28

use margins in order to fix the cursor size.
use timer to make it blink

919. By Tiago Salem Herrmann on 2012-03-05

add one more pixel in order to have a cursor 30px high and 5px top and bottom margin.

[Albert Astals Cid (aacid)]

Having a look at the Nux::TextEntry code we can find
  static const int kCursorBlinkTimeout = 400;
and
  bool TextEntry::CursorBlinkCallback(TextEntry *self)
  {
    if (self->cursor_blink_status_)
      self->ShowCursor();
    else
      self->HideCursor();

    if (--self->cursor_blink_status_ < 0)
      self->cursor_blink_status_ = 2;

    return true;
  }
and
  void TextEntry::ProcessKeyEvent(
    unsigned long event_type , /*event type*/
    unsigned long keysym , /*event keysym*/
    unsigned long state , /*event state*/
    const char* character , /*character*/
    unsigned short keyCount /*key repeat count*/)
  {
    cursor_blink_status_ = 4;

Meaning that the regular cadence of blinking is 800msec vs 400msec and when a key is typed it takes 1600msec for it to blink again

I'm not sure if we really want the 1600msec after typing stuff, but at least you should adapt the 900-300 to 800-400

review: Needs Fixing

[Albert Astals Cid (aacid)]

Gerry says we should not trouble ourselves for the 1600msec stuff on typing, so just go for the 800-400 msec change :)

[lp:~tiagosh/unity-2d/fix-dash-cursor]

920. By Tiago Salem Herrmann on 2012-03-06

change blinking interval to 800-400

[Tiago Salem Herrmann (tiagosh)]

changed to 800-400 as requested.

[Albert Astals Cid (aacid)]

With this code here my cursor is 31px height instead of 30, can you recheck you get 30?

[Tiago Salem Herrmann (tiagosh)]

measuring with gimp: http://ubuntuone.com/0ygtvxDHGkZuty7W5qrYki

[Albert Astals Cid (aacid)]

This is what i get https://imgur.com/zST1U :-/

[Albert Astals Cid (aacid)]

Actually might be related to the fact that we are not setting the font to id: searchInput just the pixelsize and we use different fonts?

[Albert Astals Cid (aacid)]

Bad me for using a non default font!

review: Approve

=== modified file 'shell/common/SearchEntry.qml'
--- shell/common/SearchEntry.qml	2012-02-28 12:30:17 +0000
+++ shell/common/SearchEntry.qml	2012-03-06 16:37:20 +0000
@@ -120,27 +120,20 @@
                 id: cursor
 
                 Rectangle {
+                    id: customCursor
                     color: "white"
-                    width: 2
-                    height: 16
-
-                    /* WARNING: that animation uses resources */
-                    /* The following animation would behave exactly like
-                       Unity if only 'search_input' could be referenced from
-                       within the cursor Component.
-                    /*
-                    SequentialAnimation on opacity {
-                        id: cursor_pulse
-                        loops: 30
-                        running: false
-                        PropertyAnimation { duration: 1000; to: 0; easing.type: Easing.InOutQuad }
-                        PropertyAnimation { duration: 1000; to: 1; easing.type: Easing.InOutQuad }
+                    anchors.top: parent.top
+                    anchors.bottom: parent.bottom
+                    anchors.topMargin: 2
+                    anchors.bottomMargin: 2
+                    width: 1
+                    Timer {
+                        interval: 800; running: true; repeat: true
+                        onTriggered: {
+                            interval = interval == 800 ? 400 : 800
+                            customCursor.visible = !customCursor.visible
+                        }
                     }
-                    Connections {
-                        target: search_input
-                        onTextChanged: cursor_pulse.running = true
-                        onActiveFocusChanged: cursor_pulse.running = search_input.activeFocus
-                    }*/
                 }
             }