[AboutSystem] underline links on mouse over

[ahartford]

Patch to italicize links on mouse over.

[ahartford]

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.

[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?

[ahartford]

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.

[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).

[ahartford]

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().

[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.

[ahartford]

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.

[ahartford]

Patch updated to draw underlines correctly when link text wraps.

[taos]

Replying to ahartford:

Such a change may warrant a new ticket.

See ticket #7491 ;-)

[ahartford]

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.

[nielx]

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

[diver]

ping :)

[mmadia]

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.

[waddlesplash]

Any progress on this?

[jessicah]

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.

[sambuddhabasu1]

I would like to work on this bug.

[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.

[sambuddhabasu1]

B_ITALIC_FACE has been used in place of B_UNDERSCORE_FACE

[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.

[sambuddhabasu1]

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

[sambuddhabasu1]

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

[jscipione]

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

[diver]

Anything to migrate to Gerrit from this ticket?

[pulkomandy]

Yes, but we should use B_UNDERSCORE_FACE and implement it (it's ok to merge this before implementing it).

[Naba7]

Can I be assigned this issue? I am a newcomer here.

[Naba7]

Cc: Naba7

[pulkomandy]

Hi, As you can see there is already a patch, but it needs the app_server (the software that draws everything to the screen) to handle B_UNDERSCORE_FACE and render the text with underline. This is maybe a little above "easy" level, but if you think you can get that part working, feel free to :)

[pulkomandy]

Change pushed to Gerrit: ​https://review.haiku-os.org/c/haiku/+/2364

[pulkomandy]

Removing "easy" from title because the easy part is already implemented (patch pending review). Now we need app_server to handle B_UNDERSCORE_FACE and that's a little more difficult.

[pulkomandy]

Fixed in hrev55255.