Code review comment for lp:~mcintire-evan/ubuntu-terminal-app/window-font-size

I left two inline comments.

As a summary, here's a diff with the changes I've proposed (line 73 is wrong - that "imports" is not required).

The big problem with this MP is the usage of QQuickView::SizeViewToRootObject.
In my opinion it creates a lot of problems (speaking in general terms) because it breaks the "Window > Contents" relationship which is at the base of a window system.

For that reason my review is a "Needs Fixing".

======

A few comments on my diff:

1) About the "Connections {}" in MainView:

It's not strictly required. I mean, in your comment above you said that you would like to reproduce that specific behaviour, but e.g. Konsole does not.

I've been a Kubuntu users in that past, and the GTK world and the KDE (Qt) world used to be very different on such things. That's why many GTK apps do things that the KDE counterpart do not, and vice versa.
Ubuntu, as a GTK distro which is moving to Qt/QML, stays in between.

So, if you want to adjust the window size after a font size change, I'd suggest you to keep the logic very simple as I do.

Things could be done better but, as long as we stay with QQuickView and don't move to QQmlApplicationEngine, that's the cleanest solution.

2) I've replaced your fontPixelSize() function with a read-only property. That's not really required, but I did it for prototyping.

3) That "onChangedFontMetricSignal" signal has a terrible name. Would you mind renaming it as "onFontMetricsChanged" in the QMLTermWidget?