search

Ketok4321 / speedtest

A graphical librespeed client written using gtk4 + libadwaita
GNU General Public License v3.0
31 stars 16 forks source link

Miscellaneous changes #26

Closed turtlegarden closed 10 months ago

turtlegarden commented 10 months ago

This pull request introduces miscellaneous UI changes to bring the app closer to the GNOME HIG. Some of these improvements include:

Most of the change diffs are simply po file updates and the blueprint formatter. There is a "broken image" in the README, but this is simply because it refers to the main branch and not my fork (once it is merged, it should work). If you have any questions, please feel free to reach out!

speedtest main UI

turtlegarden commented 10 months ago

I'm aware that the server list does not indicate when it is done loading, I was unfortunately unable to set the start button as insensitive while the server list loads.

Ketok4321 commented 10 months ago

As much as I appreciate the effort put into this PR, it's scope is way too wide for me to merge it in its current state. There are some parts of this PR that I really like, and some I'm not a huge fan of, but because of them being all in the same PR, it's much harder to cherry pick them.

The UI redesign is a really needed change, which was on my backlog for quite some time, but although this PR addresses most of the issues I had with the previous UI, such a fundamental change to the app is something I'd prefer to be in charge of myself.

Debug information in the about window is a really nice addition. I think it'd make sense for it to include a few more things like for example python or aiohttp version.

The devel toggle is neat, however it should also change the app id and icon. Also i don't think that having the manifest saved as both yaml and json is a good idea (not sure if that was intentional or not), I'm aware that GNOME builder doesn't support yaml, but having both will very likely cause issues in the future, and if I had to choose then I personally much more prefer yaml.

The swedish translation should be a separate PR.

I think it would be best to either close this PR, and submit a new PR(s) containing the debug information and development toggle, or just remove some stuff from this one.

turtlegarden commented 10 months ago

I don't have the spoons to separate things right now, I can likely work on it later. So I'll close this, and maybe get to the about window improvements sometime soon (in the next three months possibly).

yakushabb commented 6 months ago

Please do not update translations with UI changes, as it makes reviewing PRs very difficult for developers.

You can update the POT file with another commit, or separating them into another PR is also a good way to simplify things.