Opened 5 years ago

Last modified 3 years ago

#8555 assigned bug

[AboutSystem] underline links on mouse over (easy)

Reported by: diver Owned by: stpere
Priority: normal Milestone: R1
Component: Applications/AboutSystem Version: R1/Development
Keywords: Cc: adam.s.hartford@…
Blocked By: #11822 Blocking:
Has a Patch: yes Platform: All

Description


Attachments (5)

ital_actions.patch (4.5 KB) - added by Adam Hartford 5 years ago.
Patch to italicize links on mouse over.
ticket_8555.patch (16.8 KB) - added by Adam Hartford 5 years ago.
Patch updated to draw underlines correctly when link text wraps.
0001-Ticket-8555-Underline-links-on-mouse-over.-B_UNDERSC.patch (4.3 KB) - added by Sambuddha Basu 3 years ago.
B_ITALIC_FACE has been used in place of B_UNDERSCORE_FACE
0001-Ticket-8555-Underline-links-on-mouse-over.-B_UNDERSC.2.patch (4.3 KB) - added by Sambuddha Basu 3 years ago.
Underline links on mouse over. B_UNDERSCORE_FACE is not implemented so B_ITALIC_FACE has been used on mouse over
0001-Ticket-8555-Underline-links-on-mouse-over.-B_UNDERSC.3.patch (4.6 KB) - added by Sambuddha Basu 3 years ago.
Underline links on mouse over. B_UNDERSCORE_FACE is not implemented so B_ITALIC_FACE has been used on mouse over. Coding guidelines have been followed

Download all attachments as: .zip

Change History (27)

Changed 5 years ago by Adam Hartford

Attachment: ital_actions.patch added

Patch to italicize links on mouse over.

comment:1 Changed 5 years ago by Adam Hartford

Has a Patch: set

comment:2 Changed 5 years ago by Adam Hartford

I don't believe underlining is supported in Haiku right now. Instead, I have created a patch to italicize links on mouse over, which I think looks nice enough. From here it will be a simple change to underline them when it's supported, if desired.

comment:3 Changed 5 years ago by Adam Hartford

Cc: Adam Hartford added

comment:4 Changed 5 years ago by Adam Hartford

Cc: Adam Hartford removed

comment:5 Changed 5 years ago by Adam Hartford

Cc: adam.s.hartford@… added

comment:6 in reply to:  2 ; Changed 5 years ago by humdinger

Replying to ahartford:

I don't believe underlining is supported in Haiku right now.

Haven't tried your patch yet, but would underlined be the constant B_UNDERSCORE_FACE?

comment:7 in reply to:  6 Changed 5 years ago by Adam Hartford

Replying to humdinger:

Haven't tried your patch yet, but would underlined be the constant B_UNDERSCORE_FACE?

I had tried B_UNDERSCORE_FACE, but it didn't work. Then I noticed there is no underlining in StyledEdit and saw a comment from axeld in ticket #4689 mentioning that underlining is not supported in Haiku, which seems to still be true. I figured it might be appropriate to start with italics since it would just be a constant change once underlining works.

comment:8 Changed 5 years ago by axeld

Indeed, that's a missing feature we inherited from BeOS. If we wanted to have underlined links, one would have to manually draw the lines by overriding BTextView::Draw(). Or just implement it, although it might not be easy to implement it correctly for all cases (for example, Java does not support it either).

comment:9 Changed 5 years ago by Adam Hartford

I've reworked the first patch to underline links instead of italicize them. Per axeld's suggestion, the lines are drawn manually in BTextView::Draw().

comment:10 Changed 5 years ago by humdinger

There's are unneeded curly brackets in the if-statement on line 102 in the patch.

More complicated to fix is the fact that this solution apparently doesn't account for localization. Change the Locale prefs to something with a completely different font to easily spot the translated license names, e.g. Japanese or Chinese. Look for example to AntiGrain Geometry, the "BSD 3-clause". If the number of characters differ (I guess) those links aren't "mouse-overed".

I'm afraid it's back to the drawing board...

Also, maybe kDarkGrey could be a bit darker to distinguish those links from the normal text even when the mouse doesn't hover over them.

comment:11 Changed 5 years ago by Adam Hartford

From what I can see, it is expected that the translated "BSD (3-clause)" is not a hyperlink, and is unrelated to any mouse tracking. AboutSystem is trying to link the licenses to files, and there is only a "BSD (3-clause)" file in /boot/system/data/licenses, so it doesn't find a link to a localized filename and decides not to display the license as a hyperlink. It seems there needs to be a change such that the localized license links still try to point to unlocalized filenames. That, or don't translate the license names (it seems that not all of them are, currently). Such a change may warrant a new ticket.

The patch needs to be updated to draw multiple underlines when the link text wraps. I've made local changes to draw the correct number of lines, but need to figure out how to get them to start/stop at the correct character positions.

Changed 5 years ago by Adam Hartford

Attachment: ticket_8555.patch added

Patch updated to draw underlines correctly when link text wraps.

comment:12 in reply to:  11 ; Changed 5 years ago by taos

Replying to ahartford:

Such a change may warrant a new ticket.

See ticket #7491 ;-)

comment:13 in reply to:  12 Changed 5 years ago by Adam Hartford

Replying to taos:

Replying to ahartford:

Such a change may warrant a new ticket.

See ticket #7491 ;-)

Perfect! And as far as underlining is concerned, I think this is pretty much done. Any further changes like color, etc, could probably be handled separately.

comment:14 Changed 5 years ago by Niels Sascha Reedijk

Gentle reminder that this patch is ready to get a review...

comment:15 Changed 5 years ago by diver

ping :)

comment:16 Changed 5 years ago by Matt Madia

Owner: changed from Nobody to stpere
Status: newassigned

As mentioned when talking with stpere, the same effect for visually stressing a clickable URL should be the same in People (#3825) and in AboutSystem.

comment:17 Changed 4 years ago by waddlesplash

Any progress on this?

comment:18 Changed 3 years ago by Jessica

I've had a go at applying this, but some of the lines aren't drawn correctly. There are several cases where the lines are either too short or too long. It also doesn't handle spaces in line-wrapped instances well either (the trailing space shouldn't be underlined for instance).

It may only be a few pixels either way, but it just doesn't look nice.

comment:19 Changed 3 years ago by Sambuddha Basu

I would like to work on this bug.

comment:20 Changed 3 years ago by PulkoMandy

I would prever if we implemented B_UNDERSCORE_FACE rather than using ad hoc code in all apps which need it. This way we can writeand debug the code only once.

Changed 3 years ago by Sambuddha Basu

B_ITALIC_FACE has been used in place of B_UNDERSCORE_FACE

comment:21 Changed 3 years ago by PulkoMandy

This looks good so far (didn't test it yet). There are some possible improvements however:

const ActionInfo* _ActionInfoAt(const BPoint& where) const; should be declared in the "methods" part of the class (next to _ActionAt, currently you have it in the "fields" part of the class.

view->SetFontAndColor(startOffset, endOffset, &font); in both MouseOver and MouseAway, you can use the "mask" parameter of SetFontAndColor to only change the face. This makes the code simpler:

BFont font;
font.SetFace(B_ITALICS_FACE);
view->SetFontAndColor(startOffset, endOffset, &font, B_FONT_FACE);

It makes sure nothing other than the face is changed, so the size, family, etc. are preserved.

Changed 3 years ago by Sambuddha Basu

Underline links on mouse over. B_UNDERSCORE_FACE is not implemented so B_ITALIC_FACE has been used on mouse over

Changed 3 years ago by Sambuddha Basu

Underline links on mouse over. B_UNDERSCORE_FACE is not implemented so B_ITALIC_FACE has been used on mouse over. Coding guidelines have been followed

comment:22 Changed 3 years ago by John

Blocked By: 11822 added

(In #11822) B_UNDERSCORE_FACE has to be implemented before we can fix #8555

Note: See TracTickets for help on using tickets.