Visual settings: Font family order

[VesnaT]

Issue

Implements comment from #4828.

Fonts start with a list of (exotic) fonts whose names start with "." (on macOS). Well, one of them is the default, San Francisco (shown as .SF). I propose the following ordering:
- default family (.SF on macOS), but without the leading period.
- all fonts that don't begin with period.
- all fonts that begin with period (assuming they are exotic system fonts).
Here I of course assume that this order is not imposed by Qt and is possible to change. (I haven't checked the code.)

Description of changes

• sort font families

Includes

• Code changes
• Tests
• Documentation

[codecov]

Codecov Report

Merging #4892 into master will decrease coverage by 0.03%.
The diff coverage is 73.91%.

@@            Coverage Diff             @@
##           master    #4892      +/-   ##
==========================================
- Coverage   84.50%   84.47%   -0.04%     
==========================================
  Files         283      283              
  Lines       58306    58367      +61     
==========================================
+ Hits        49270    49304      +34     
- Misses       9036     9063      +27     

[janezd]

Perhaps it would be better to put not just the default family but also the related fonts at the top. It is impossible to guess which part of the name is the actual name and which part determines the shape, so I simply took the first word. Please take a look at the commit I made.

More importantly, I'd propose a change to the dialog to get rid of the leading dots. The commit that I added requires adding the following code to visual_settings_dlg.

class FontList(list):
    pass


@_add_control.register(FontList)
def _(values: FontList, value: str, key: KeyType, signal: Callable) \
        -> QComboBox:
    class FontModel(PyListModel):
        def data(self, index, role=Qt.DisplayRole):
            value = super().data(index, role)
            if role in (Qt.DisplayRole, Qt.EditRole) \
                and isinstance(value, str) and value[0] == ".":
                value = value[1:]
            return value

    combo = QComboBox()
    combo.setModel(FontModel(values))
    combo.setCurrentIndex(values.index(value))
    combo.currentIndexChanged.connect(lambda i: signal.emit(key, values[i]))
    return combo

Please comment. Note also that the separator doesn't work. The check is self.Separator in PyListModel does not work because the separator misteriously turns into a different object. Are initial settings deep-copied somewhere?

[VesnaT]

Perhaps it would be better to put not just the default family but also the related fonts at the top.

I don't like the way we are complicating the code over the font families.

More importantly, I'd propose a change to the dialog to get rid of the leading dots. The commit that I added requires adding the following code to visual_settings_dlg.

I'd rather not put font-specific code into a base module such as visual_settings_dlg. I don't have an alternative solution (yet), but I cannot agree with this one.
EDIT: I suppose values could be an instance of a QAbstractListModel.

Are initial settings deep-copied somewhere?

initial_settings are deep-copied in projection widgets.

[janezd]

I haven't solved the mystery of separators. This works now, but it imports a private class (a delegate). Could be fixed (by making it non-private).

class FontList(list):
    pass


@_add_control.register(FontList)
def _(values: FontList, value: str, key: KeyType, signal: Callable) \
        -> QComboBox:
    class FontModel(QStringListModel):
        def data(self, index, role=Qt.DisplayRole):
            if role == Qt.AccessibleDescriptionRole \
                    and super().data(index, Qt.DisplayRole) == "":
                return "separator"

            value = super().data(index, role)
            if role in (Qt.DisplayRole, Qt.EditRole) and value.startswith("."):
                value = value[1:]
            return value

        def flags(self, index):
            if index.data(Qt.DisplayRole) == "separator":
                return Qt.NoItemFlags
            else:
                return super().flags(index)

    combo = QComboBox()
    model = FontModel(values)
    combo.setModel(model)
    combo.setCurrentIndex(values.index(value))
    combo.currentIndexChanged.connect(lambda i: signal.emit(key, values[i]))
    combo.setItemDelegate(_ComboBoxListDelegate())
    return combo

Perhaps it would be better to put not just the default family but also the related fonts at the top.
I don't like the way we are complicating the code over the font families.

Why do you think it's complicated? It's just a model that removes a leading period.

I'd rather not put font-specific code into a base module such as visual_settings_dlg. I don't have an alternative solution (yet), but I cannot agree with this one.

This one is harder for me to argue about because this is your concept and you know how it's supposed to work. But won't almost every customizable widget allow changing the font? Why is it specific? Isn't this one of basic things that (w|sh)ould be handled here?

EDIT: I suppose values could be an instance of a QAbstractListModel.

Thanks for the hint -- it helped me fix the separator. Not QAbstractListModel, but QStringListModel.

[VesnaT]

Why do you think it's complicated? It's just a model that removes a leading period.

Exactly, 30 lines of code for the leading period (and the separator.) :)

[janezd]

Why do you think it's complicated? It's just a model that removes a leading period.

Exactly, 30 lines of code for the leading period (and the separator.) :)

Sorry, did you say complicated or long? :)

[janezd]

@PrimozGodec, we discussed this again with @VesnaT. The model (in orange-widget-base repository) has to stay. The question is whether to remove fonts with leading dot, except the system font, and whether the system font should be renamed to something like "Default font". The latter may not be appropriate, because this is not a default font in any way, except that it's used for dialogs. (E.g. default font in Word is Calibri, but Orange does not have a default.)

At the end, we guess it'd be better to just keep both PRs as they are: keep the fonts with a leading dot, but don't show the dots.

[PrimozGodec]

I am also ok with the solution that you suggest (keep the fonts with a leading dot, but don't show the dots). In the current implementation, I still see dots in names.
It seems that available_font_families is never used and so FontList is never used. Do I see anything wrong?

[janezd]

It seems that available_font_families is never used and so FontList is never used. Do I see anything wrong?

You're right. It was a result of botched rebase to master. Even now, if I try to rebase, it shows conflicts (which github doesn't) and removes the call to available_font_families.

[markotoplak]

When merging this PR we forgot to update dependencies again. The required widget base should be 4.8.1. @PrimozGodec