After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 738671 - Message divider overlap text
Message divider overlap text
Status: RESOLVED FIXED
Product: polari
Classification: Applications
Component: general
3.14.x
Other Linux
: Normal major
: ---
Assigned To: Polari maintainers
Polari maintainers
Depends on:
Blocks:
 
 
Reported: 2014-10-17 08:21 UTC by Nicola
Modified: 2015-04-06 09:58 UTC
See Also:
GNOME target: ---
GNOME version: ---

Attachments
image that show the problem (426.83 KB, image/png)
2014-10-17 08:21 UTC, Nicola 		  Details
Make line indicator dependant on line spacing (1.61 KB, patch)
2015-04-01 13:01 UTC, Bastian Ilsø 		none Details | Review
screenshot of line indicator after patch (144.86 KB, image/png)
2015-04-01 13:04 UTC, Bastian Ilsø 		  Details
Make line indicator dependant on line spacing (1.61 KB, patch)
2015-04-01 13:44 UTC, Bastian Ilsø 		reviewed Details | Review
Make line indicator dependent on line spacing (1.60 KB, patch)
2015-04-01 19:15 UTC, Bastian Ilsø 		accepted-commit_now Details | Review

Description Nicola 2014-10-17 08:21:32 UTC 
Created attachment 288727 [details]
image that show the problem

This is with polari 3.14.1
Comment 1 Bastian Ilsø 2015-04-01 13:01:26 UTC 
Created attachment 300744 [details] [review]
Make line indicator dependant on line spacing

The position of the 'new messages' line indicator was based on a constant 'LINE_INDICATOR_OFFSET'. The line indicators position is now dependant on the linespacing used by the GtkTextView.
Comment 2 Bastian Ilsø 2015-04-01 13:04:26 UTC 
Created attachment 300745 [details]
screenshot of line indicator after patch

attached screenshot shows the result I'm getting with above attached patch. I'm testing on a hi-dpi screen and the patch seem to work for me.
Comment 3 Bastian Ilsø 2015-04-01 13:44:19 UTC 
Created attachment 300749 [details] [review]
Make line indicator dependant on line spacing

The position of the 'new messages' line indicator was based on a constant 'LINE_INDICATOR_OFFSET'. The line indicators position is now dependant on the linespacing used by the GtkTextView.
Comment 4 Florian Müllner 2015-04-01 19:06:06 UTC 
Review of attachment 300749 [details] [review]:

Minor nits:
Commit message body needs wrapping (git-log etc. doesn't do that automatically), and I'd use the 'dependent' spelling in the subject (both are synonyms in American English, but it's a bit confusing when used to British spelling).

::: src/chatView.js
@@ +65,3 @@
 
+        let lineSpace = (this.get_pixels_above_lines(this) + 
+                         this.get_pixels_below_lines(this)) / 2; 

Trailing whitespace on both lines
Comment 5 Bastian Ilsø 2015-04-01 19:15:46 UTC 
Created attachment 300774 [details] [review]
Make line indicator dependent on line spacing

The position of the 'new messages' line indicator
was based on a constant 'LINE_INDICATOR_OFFSET'.
The line indicators position is now dependent on
the linespacing used by the GtkTextView.
Comment 6 Florian Müllner 2015-04-01 20:24:02 UTC 
Review of attachment 300774 [details] [review]:

Thanks, go ahead ...