This looks better and allows better separation between header and text paragraph. But I'm not sure if we consider this somewhat abuse of the selected state - seems a bit like a hack?

I'll let @Brecht Van Lommel (brecht) be the judge.

I guess the UI_SELECT_DRAW flag is just for these purposes.

UI_SELECT = (1 << 0), /* use when the button is pressed */
UI_SELECT_DRAW = (1 << 5), /* Display selected, doesn't impact interaction. */

We also already use this "hack" for Splash Screen to draw version number.

Interesting.

Although could we get the same result by adding a new uiBut.flag instead? As in follow the pattern of UI_BUT_REDALERT (UI_interface.h) as there is also a similar UI_BUT_INACTIVE too. LIke "UI_BUT_TEXTHI" or similar. then check for that in interface_widgets.c widget_state() and set the text color to use TH_TEXT_HI.

It looks like you'd also want to alter uiItemL so that it returns a pointer to the uiBut it creates, like the others.

Then you could do something like:

uiBut *title = uiItemL(layout, title, ICON_ERROR);
UI_but_flag_enable(title, UI_BUT_TEXTHI);

This is overkill IMHO.
Will this white color be themeable?
What if my dialog box is light/white, the text will be unreadable?

If I can't change that white color, that's not good.

@ThinkingPolygons (ThinkingPolygons) wrote:
This is overkill IMHO.
Will this white color be themeable?
What if my dialog box is light/white, the text will be unreadable?
If I can't change that white color, that's not good.

Why not test the patch and find out for yourself?

But yes, he should not have said "white" but instead mentioned that it used Menu Back "selected" color rather than the usual "text" color.

You can add an uiItemL_ex function with an extra const bool highlight argument. This can then be extended as needed.

uiItem functions do not return uiBut* pointers, they're meant to abstract away individual buttons.

This is just drawing a label with the Text Highlight color from the theme, which is perfectly fine.

What's poor here is the name UI_SELECT_DRAW, it could be renamed to UI_TEXT_HIGHLIGHT to make more clear what it does.

In D6934#164786, @Harley Acheson (harley) wrote:

Why not test the patch and find out for yourself?

I would if I knew how.

But yes, he should not have said "white" but instead mentioned that it used Menu Back "selected" color rather than the usual "text" color.

Alright, thanks for the explanation. I was worried a bit. lol

In D6934#164788, @ThinkingPolygons (ThinkingPolygons) wrote:

I would if I knew how.

If you want I'll send you the needed infos to your devtalk account.

@Brecht Van Lommel (brecht) - What's poor here is the name UI_SELECT_DRAW, it could be renamed to UI_TEXT_HIGHLIGHT to make more clear what it does.

The current uses of UI_SELECT_DRAW make a bit more sense as it is altering press-able buttons. Would be nice to keep this behavior separate so we could later make other (possible) changes to those types of buttons, like outline color, etc.

Is it a thought to go with your uiItemL_ex() idea but use a new uiBut flag like UI_TEXT_HIGHLIGHT? That way it would be a bit more targeted. And we could even use it for non-label things if we really wanted to.

@Debuk (Debuk) That would be nice. Thank you.

@Brecht Van Lommel (brecht)

uiItem functions do not return uiBut* pointers, they're meant to abstract away individual buttons.

The uiItemL_ returns a pointer. The current code:

static uiBut *uiItemL_(uiLayout *layout, const char *name, int icon)
{
  ...
}

void uiItemL(uiLayout *layout, const char *name, int icon)
{
  uiItemL_(layout, name, icon);
}

I just added:

void uiItemL_select(uiLayout *layout, const char *name, int icon)
{
  uiBut *but = uiItemL_(layout, name, icon);
  UI_but_flag_enable(but, UI_SELECT_DRAW);
}

It's a minimalistic code, and it's easy to use.

uiItemL_select(layout, title, ICON_NONE);

What's poor here is the name UI_SELECT_DRAW, it could be renamed to UI_TEXT_HIGHLIGHT to make more clear what it does.

The UI_SELECT_DRAW already exists, it's not my idea.

I'm not sure it will be used much so I don't see the need to do anything complicated for now.

In the theme editor, this (color used) is called "Selected", so there is a reason to call it "Selected" instead of "Highlighted".

In any case, regardless of how it will be implemented internally, it is convenient to have a uiItemL_select() wrapper to make it easy to use. That's exactly what's done in this patch.

Hey, if you are going to go through the trouble of adding a uiItemL_ex() in that way (called after uiItemL_) then it might be nice to support an "alignment" argument.

As in you could center text in a block if you wanted to, by just removing UI_BUT_TEXT_LEFT and UI_BUT_TEXT_RIGHT. Unfortunately we don't have a UI_BUT_TEXT_CENTER defined as that is just the default. But a new enum could be defined for passing to it. Could come in handy for titling sometimes (like on a popover) if a horizontal rule was added afterward.

Just a thought anyway. But probably off-topic.