Differential D20880

Reduce the size of busy indicators
ClosedPublic
Actions

Authored by Nilsolm on Apr 28 2019, 4:26 PM.

Details

Reviewers

mgallien
ngraham

Group Reviewers

Elisa
VDG

Commits

R255:bc9b05ca4e55: Reduce the size of busy indicators

Summary

As reported in 406940, the busy indicators that show up when data is being loaded are a bit too big at the moment. These changes reduce their size by anchoring them at the center instead of having them fill the available space (see example screenshots below for comparison).

BUG: 406940

Track view before change:

Track view after change:

Grid view before change:

Grid view after change:

Test Plan

This is just a minor UI tweak. No behavioural changes.

Diff Detail

Repository

R255 Elisa

Lint

Automatic diff as part of commit; lint not applicable.

Unit

Automatic diff as part of commit; unit tests not applicable.

Nilsolm requested review of this revision.Apr 28 2019, 4:26 PM

Nilsolm created this revision.

Thanks very much for the contribution! Nice first patch, too.

I'd say something in between those two sizes would be ideal. The current one is too big, but the proposed new size is too small IMO. I think if you doubled or tripled the size of the new one, it would be just right. Here's an example of how Discover modifies the size of its busy indicator: https://cgit.kde.org/discover.git/tree/discover/qml/UpdatesPage.qml#n140

This revision now requires changes to proceed.Apr 29 2019, 4:32 PM

Thanks for the hint! I did manage to make it work by using Kirigami's gridUnit. How about this size?

Track view

Grid view

Nilsolm updated this revision to Diff 57250.Apr 30 2019, 11:47 AM

Thanks for your work.
Please can you add the required version for Kirigami in the CMakeLists.txt file ?
Apart from that, this is fine for me

Nice! Can you provide your full name and email address so we can land this with correct authorship information once @mgallien has accepted it?

@mgallien looks like we already have Kirigami listed in CMakeLists.txt, and I think Units has been around in it since forever-ish.

This revision is now accepted and ready to land.Apr 30 2019, 12:05 PM

In D20880#458546, @ngraham wrote:

Nice! Can you provide your full name and email address so we can land this with correct authorship information once @mgallien has accepted it?

@mgallien looks like we already have Kirigami listed in CMakeLists.txt, and I think Units has been around in it since forever-ish.

Yes but the import requires at least the 2.5 version. It may work with any version 2 but the qml engine will refuse it due to the version constraint.

In D20880#458531, @mgallien wrote:

Thanks for your work.
Please can you add the required version for Kirigami in the CMakeLists.txt file ?
Apart from that, this is fine for me

Gladly. That would just mean setting the required Kirigami version to 2.5.0 instead of using the required KF5 version, right?

In D20880#458546, @ngraham wrote:

Nice! Can you provide your full name and email address so we can land this with correct authorship information once @mgallien has accepted it?

@mgallien looks like we already have Kirigami listed in CMakeLists.txt, and I think Units has been around in it since forever-ish.

Sure, name is David Koermendi, email is nilsolm(at)vivaldi(dot)net.

This should do it, I hope

In D20880#458661, @Nilsolm wrote:

This should do it, I hope

I will check on my laptop and will accept and land your patch. Thanks for your contribution !

mgallien set the repository for this revision to R255 Elisa.Apr 30 2019, 3:18 PM

mgallien added a project: Elisa.

In D20880#458660, @Nilsolm wrote:

In D20880#458531, @mgallien wrote:

Thanks for your work.
Please can you add the required version for Kirigami in the CMakeLists.txt file ?
Apart from that, this is fine for me

Gladly. That would just mean setting the required Kirigami version to 2.5.0 instead of using the required KF5 version, right?

In fact, kirigami2 has the same version than other KF5 frameworks. I will land your patch without the CMakeLists.txt change. Sorry for that.

In D20880#458546, @ngraham wrote:

Nice! Can you provide your full name and email address so we can land this with correct authorship information once @mgallien has accepted it?

@mgallien looks like we already have Kirigami listed in CMakeLists.txt, and I think Units has been around in it since forever-ish.

Sure, name is David Koermendi, email is nilsolm(at)vivaldi(dot)net.

mgallien accepted this revision.Apr 30 2019, 3:37 PM

Closed by commit R255:bc9b05ca4e55: Reduce the size of busy indicators (authored by Nilsolm, committed by mgallien). · Explain WhyApr 30 2019, 3:40 PM

This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

			Path	Packages
M			src/qml/AlbumView.qml (7 lines)
M			src/qml/DataGridView.qml (7 lines)
M			src/qml/FrequentlyPlayedTracks.qml (7 lines)
M			src/qml/GridBrowserDelegate.qml (8 lines)
M			src/qml/RecentlyPlayedTracks.qml (7 lines)
M			src/qml/TracksView.qml (7 lines)

Diff	ID	Base	Description	Created	Lint	Unit
Base			Base
Diff 1	57139			Apr 28 2019, 4:04 PM	★	★
Diff 2	57250			Apr 30 2019, 11:47 AM	★	★
Diff 3	57272			Apr 30 2019, 2:12 PM	★	★
Diff 4	57276	ae0be38	R255:bc9b05ca4e555e19e07340cb6122e9cd63a37d26	Apr 30 2019, 3:40 PM	★	★