Merge lp:~lukas-kde/qtubuntu/ubuntuIconsFonts into lp:qtubuntu

Proposed by Lukáš Tinkl on 2017-03-28
Status: Merged
Approved by: Daniel d'Andrada on 2017-03-29
Approved revision: 388
Merged at revision: 392
Proposed branch: lp:~lukas-kde/qtubuntu/ubuntuIconsFonts
Merge into: lp:qtubuntu
Diff against target: 236 lines (+77/-53)
6 files modified
src/shared/ubuntutheme.h (+63/-0)
src/ubuntuappmenu/theme.cpp (+3/-20)
src/ubuntuappmenu/theme.h (+4/-7)
src/ubuntuappmenu/ubuntuappmenu.pro (+2/-1)
src/ubuntumirclient/qmirclientintegration.cpp (+3/-24)
src/ubuntumirclient/ubuntumirclient.pro (+2/-1)
To merge this branch: bzr merge lp:~lukas-kde/qtubuntu/ubuntuIconsFonts
Reviewer Review Type Date Requested Status
Daniel d'Andrada (community) 2017-03-28 Approve on 2017-03-29
Unity8 CI Bot continuous-integration Approve on 2017-03-29
Review via email: mp+321178@code.launchpad.net

Commit message

Use the correct font (Ubuntu family) and icon theme (suru)

Description of the change

Use the correct font (Ubuntu family) and icon theme (suru)

Together with the logicalDPI branch (https://code.launchpad.net/~dandrader/qtubuntu/logicalDpi/+merge/320940) looks like this: https://imgur.com/vTmO85M

To post a comment you must log in.
Unity8 CI Bot (unity8-ci-bot) wrote :

PASSED: Continuous integration, rev:385
https://unity8-jenkins.ubuntu.com/job/lp-qtubuntu-ci/216/
Executed test runs:
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build/4733
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/4761
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/4584
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/4584/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/4584
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/4584/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/4584
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/4584/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/4584
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/4584/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/4584
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/4584/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/4584
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/4584/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-qtubuntu-ci/216/rebuild

review: Approve (continuous-integration)
Daniel d'Andrada (dandrader) wrote :

Please update the copyright header of the modified files.

Daniel d'Andrada (dandrader) wrote :

"""
private:
    QFont systemFont, fixedFont;
"""

Should follow the project's naming convention for member variables. Eg:
s/systemFont/mSystemFont

review: Needs Fixing
Daniel d'Andrada (dandrader) wrote :

Other than that it looks good and works as expected.

386. By Lukáš Tinkl on 2017-03-29

update (c), prefix private member with "m"

Lukáš Tinkl (lukas-kde) wrote :

> Please update the copyright header of the modified files.

Done

Lukáš Tinkl (lukas-kde) wrote :

> """
> private:
> QFont systemFont, fixedFont;
> """
>
> Should follow the project's naming convention for member variables. Eg:
> s/systemFont/mSystemFont

Fixed

Unity8 CI Bot (unity8-ci-bot) wrote :

PASSED: Continuous integration, rev:386
https://unity8-jenkins.ubuntu.com/job/lp-qtubuntu-ci/217/
Executed test runs:
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build/4760
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/4788
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/4611
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/4611/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/4611
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/4611/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/4611
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/4611/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/4611
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/4611/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/4611
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/4611/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/4611
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/4611/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-qtubuntu-ci/217/rebuild

review: Approve (continuous-integration)
Daniel d'Andrada (dandrader) wrote :

Looking at the diff again, don't know why I missed that before:
Would it be possible for ubuntuappmenu and ubuntumirclient to share the very same theme class? Better than copy-pasting code around.

review: Needs Information
Lukáš Tinkl (lukas-kde) wrote :

> Looking at the diff again, don't know why I missed that before:
> Would it be possible for ubuntuappmenu and ubuntumirclient to share the very
> same theme class? Better than copy-pasting code around.

Yeah, was wondering the same, I can give it a try

387. By Lukáš Tinkl on 2017-03-29

factor out the common QPlatformTheme bits into a shared header file

Lukáš Tinkl (lukas-kde) wrote :

> Looking at the diff again, don't know why I missed that before:
> Would it be possible for ubuntuappmenu and ubuntumirclient to share the very
> same theme class? Better than copy-pasting code around.

Alright, the QPlatformTheme code is now shared between the two plugins (the common stuff)

Daniel d'Andrada (dandrader) wrote :

Much better. Last bit:
s/UbuntuIconTheme/UbuntuTheme

review: Needs Fixing
Unity8 CI Bot (unity8-ci-bot) wrote :

PASSED: Continuous integration, rev:387
https://unity8-jenkins.ubuntu.com/job/lp-qtubuntu-ci/220/
Executed test runs:
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build/4770
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/4798
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/4621
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/4621/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/4621
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/4621/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/4621
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/4621/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/4621
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/4621/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/4621
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/4621/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/4621
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/4621/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-qtubuntu-ci/220/rebuild

review: Approve (continuous-integration)
388. By Lukáš Tinkl on 2017-03-29

s/UbuntuIconTheme/UbuntuTheme

Lukáš Tinkl (lukas-kde) wrote :

> Much better. Last bit:
> s/UbuntuIconTheme/UbuntuTheme

Done

Unity8 CI Bot (unity8-ci-bot) wrote :

PASSED: Continuous integration, rev:388
https://unity8-jenkins.ubuntu.com/job/lp-qtubuntu-ci/221/
Executed test runs:
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build/4771
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/4799
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/4622
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/4622/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/4622
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/4622/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/4622
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/4622/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/4622
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/4622/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/4622
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/4622/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/4622
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/4622/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-qtubuntu-ci/221/rebuild

review: Approve (continuous-integration)
Daniel d'Andrada (dandrader) wrote :

Thanks

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== added directory 'src/shared'
2=== added file 'src/shared/ubuntutheme.h'
3--- src/shared/ubuntutheme.h 1970-01-01 00:00:00 +0000
4+++ src/shared/ubuntutheme.h 2017-03-29 14:17:03 +0000
5@@ -0,0 +1,63 @@
6+/*
7+ * Copyright (C) 2016-2017 Canonical, Ltd.
8+ *
9+ * This program is free software: you can redistribute it and/or modify it under
10+ * the terms of the GNU Lesser General Public License version 3, as published by
11+ * the Free Software Foundation.
12+ *
13+ * This program is distributed in the hope that it will be useful, but WITHOUT
14+ * ANY WARRANTY; without even the implied warranties of MERCHANTABILITY,
15+ * SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
16+ * Lesser General Public License for more details.
17+ *
18+ * You should have received a copy of the GNU Lesser General Public License
19+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
20+ */
21+
22+#include <QVariant>
23+#include <QtPlatformSupport/private/qgenericunixthemes_p.h>
24+
25+class UbuntuTheme : public QGenericUnixTheme
26+{
27+public:
28+ UbuntuTheme()
29+ : mSystemFont(QStringLiteral("Ubuntu Regular"), 10),
30+ mFixedFont(QStringLiteral("Ubuntu Mono Regular"), 13)
31+ {
32+ mSystemFont.setStyleHint(QFont::System);
33+ mFixedFont.setStyleHint(QFont::TypeWriter);
34+ }
35+ ~UbuntuTheme() = default;
36+
37+ QVariant themeHint(ThemeHint hint) const override
38+ {
39+ switch (hint) {
40+ case QPlatformTheme::SystemIconThemeName: {
41+ QByteArray iconTheme = qgetenv("QTUBUNTU_ICON_THEME");
42+ if (iconTheme.isEmpty()) {
43+ return QStringLiteral("suru");
44+ } else {
45+ return iconTheme;
46+ }
47+ }
48+ default:
49+ break;
50+ }
51+ return QGenericUnixTheme::themeHint(hint);
52+ }
53+
54+ const QFont *font(Font type) const override
55+ {
56+ switch (type) {
57+ case QPlatformTheme::SystemFont:
58+ return &mSystemFont;
59+ case QPlatformTheme::FixedFont:
60+ return &mFixedFont;
61+ default:
62+ return nullptr;
63+ }
64+ }
65+
66+private:
67+ QFont mSystemFont, mFixedFont;
68+};
69
70=== modified file 'src/ubuntuappmenu/theme.cpp'
71--- src/ubuntuappmenu/theme.cpp 2017-02-16 13:33:03 +0000
72+++ src/ubuntuappmenu/theme.cpp 2017-03-29 14:17:03 +0000
73@@ -1,5 +1,5 @@
74 /*
75- * Copyright (C) 2016 Canonical, Ltd.
76+ * Copyright (C) 2016-2017 Canonical, Ltd.
77 *
78 * This program is free software: you can redistribute it and/or modify it under
79 * the terms of the GNU Lesser General Public License version 3, as published by
80@@ -34,29 +34,12 @@
81
82 }
83
84-UbuntuAppMenuTheme::UbuntuAppMenuTheme()
85+UbuntuAppMenuTheme::UbuntuAppMenuTheme():
86+ UbuntuTheme()
87 {
88 qCDebug(ubuntuappmenu, "UbuntuAppMenuTheme::UbuntuAppMenuTheme() - useLocalMenu=%s", useLocalMenu() ? "true" : "false");
89 }
90
91-UbuntuAppMenuTheme::~UbuntuAppMenuTheme()
92-{
93-}
94-
95-QVariant UbuntuAppMenuTheme::themeHint(ThemeHint hint) const
96-{
97- if (hint == QPlatformTheme::SystemIconThemeName) {
98- QByteArray iconTheme = qgetenv("QTUBUNTU_ICON_THEME");
99- if (iconTheme.isEmpty()) {
100- return QVariant(QStringLiteral("ubuntu-mobile"));
101- } else {
102- return QVariant(QString(iconTheme));
103- }
104- } else {
105- return QGenericUnixTheme::themeHint(hint);
106- }
107-}
108-
109 QPlatformMenuItem *UbuntuAppMenuTheme::createPlatformMenuItem() const
110 {
111 if (useLocalMenu()) return QGenericUnixTheme::createPlatformMenuItem();
112
113=== modified file 'src/ubuntuappmenu/theme.h'
114--- src/ubuntuappmenu/theme.h 2017-02-16 13:33:03 +0000
115+++ src/ubuntuappmenu/theme.h 2017-03-29 14:17:03 +0000
116@@ -1,5 +1,5 @@
117 /*
118- * Copyright (C) 2016 Canonical, Ltd.
119+ * Copyright (C) 2016-2017 Canonical, Ltd.
120 *
121 * This program is free software: you can redistribute it and/or modify it under
122 * the terms of the GNU Lesser General Public License version 3, as published by
123@@ -17,17 +17,14 @@
124 #ifndef UBUNTU_THEME_H
125 #define UBUNTU_THEME_H
126
127-#include <QtPlatformSupport/private/qgenericunixthemes_p.h>
128+#include "../shared/ubuntutheme.h"
129
130-class UbuntuAppMenuTheme : public QGenericUnixTheme
131+class UbuntuAppMenuTheme : public UbuntuTheme
132 {
133 public:
134 static const char* name;
135 UbuntuAppMenuTheme();
136- virtual ~UbuntuAppMenuTheme();
137-
138- // From QPlatformTheme
139- QVariant themeHint(ThemeHint hint) const override;
140+ ~UbuntuAppMenuTheme() = default;
141
142 // For the menus
143 QPlatformMenuItem* createPlatformMenuItem() const override;
144
145=== modified file 'src/ubuntuappmenu/ubuntuappmenu.pro'
146--- src/ubuntuappmenu/ubuntuappmenu.pro 2017-03-06 15:28:00 +0000
147+++ src/ubuntuappmenu/ubuntuappmenu.pro 2017-03-29 14:17:03 +0000
148@@ -23,7 +23,8 @@
149 menuregistrar.h \
150 registry.h \
151 themeplugin.h \
152- qtubuntuextraactionhandler.h
153+ qtubuntuextraactionhandler.h \
154+ ../shared/ubuntutheme.h
155
156 SOURCES += \
157 theme.cpp \
158
159=== modified file 'src/ubuntumirclient/qmirclientintegration.cpp'
160--- src/ubuntumirclient/qmirclientintegration.cpp 2017-02-07 15:37:20 +0000
161+++ src/ubuntumirclient/qmirclientintegration.cpp 2017-03-29 14:17:03 +0000
162@@ -1,6 +1,6 @@
163 /****************************************************************************
164 **
165-** Copyright (C) 2014-2016 Canonical, Ltd.
166+** Copyright (C) 2014-2017 Canonical, Ltd.
167 ** Contact: https://www.qt.io/licensing/
168 **
169 ** This file is part of the plugins of the Qt Toolkit.
170@@ -50,6 +50,7 @@
171 #include "qmirclientnativeinterface.h"
172 #include "qmirclientscreen.h"
173 #include "qmirclientwindow.h"
174+#include "../shared/ubuntutheme.h"
175
176 // Qt
177 #include <QFileInfo>
178@@ -61,7 +62,6 @@
179 #include <QtPlatformSupport/private/qgenericunixfontdatabase_p.h>
180 #include <QtPlatformSupport/private/qgenericunixeventdispatcher_p.h>
181 #include <QtPlatformSupport/private/qeglpbuffer_p.h>
182-#include <QtPlatformSupport/private/qgenericunixthemes_p.h>
183 #include <QtPlatformSupport/private/bridge_p.h>
184 #include <QOpenGLContext>
185 #include <QOffscreenSurface>
186@@ -71,27 +71,6 @@
187 #include <ubuntu/application/id.h>
188 #include <ubuntu/application/options.h>
189
190-
191-class UbuntuIconTheme : public QGenericUnixTheme
192-{
193-public:
194- UbuntuIconTheme() {}
195-
196- // From QPlatformTheme
197- QVariant themeHint(ThemeHint hint) const override {
198- if (hint == QPlatformTheme::SystemIconThemeName) {
199- QByteArray iconTheme = qgetenv("QTUBUNTU_ICON_THEME");
200- if (iconTheme.isEmpty()) {
201- return QVariant(QStringLiteral("ubuntu-mobile"));
202- } else {
203- return QVariant(QString(iconTheme));
204- }
205- } else {
206- return QGenericUnixTheme::themeHint(hint);
207- }
208- }
209-};
210-
211 static void resumedCallback(const UApplicationOptions */*options*/, void *context)
212 {
213 auto integration = static_cast<QMirClientClientIntegration*>(context);
214@@ -351,7 +330,7 @@
215 QPlatformTheme* QMirClientClientIntegration::createPlatformTheme(const QString& name) const
216 {
217 Q_UNUSED(name);
218- return new UbuntuIconTheme;
219+ return new UbuntuTheme;
220 }
221
222 QVariant QMirClientClientIntegration::styleHint(StyleHint hint) const
223
224=== modified file 'src/ubuntumirclient/ubuntumirclient.pro'
225--- src/ubuntumirclient/ubuntumirclient.pro 2017-02-07 15:37:20 +0000
226+++ src/ubuntumirclient/ubuntumirclient.pro 2017-03-29 14:17:03 +0000
227@@ -48,7 +48,8 @@
228 qmirclientscreen.h \
229 qmirclientwindow.h \
230 qmirclientlogging.h \
231- qmirclientappstatecontroller.h
232+ qmirclientappstatecontroller.h \
233+ ../shared/ubuntutheme.h
234
235 OTHER_FILES += \
236 ubuntumirclient.json

Subscribers

People subscribed via source and target branches